[PATCH] Fix ICE with movstrictqi (PR target/92791)

[Uros Bizjak]

On Thu, Dec 5, 2019 at 9:21 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The huge LTO testcase in the PR ICEs, because in a function
> where optimize_function_for_speed_p (cfun) and when targetting
> -march=i686 optab_handler (movstrict_optab, E_QImode) is
> CODE_FOR_movstrictqi, but when the *movstrictqi instruction is being matched
> during vregs pass, the condition:
> !TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun)
> FAILs.  I haven't been able to construct a small testcase that reproduces
> that, but in the PR is another one which reproduces the opposite,
> where optab_handler (movstrict_optab, E_QImode) is CODE_FOR_nothing
> in function where optimize_function_for_size_p (cfun) is true and thus
> we unnecessarily create larger code.
>
> From the debugging, seems nothing guarantees that functions with
> different optimize_function_for_{size,speed}_p (cfun) will use different
> this_fn_optabs in which init_all_optabs records which patterns are disabled
> and which are enabled.
>
> The documentation says:
> @cindex named patterns and conditions
> For a named pattern, the condition may not depend on the data in the
> insn being matched, but only the target-machine-type flags.  The compiler
> needs to test these conditions during initialization in order to learn
> exactly which named instructions are available in a particular run.
>
> and while the condition of movstrict<mode> doesn't depend on the data in the
> instruction, optimize_function_for_*_p (cfun) is not target-machine-type
> flags only either.  Furthermore, seems movstrict<mode> is the only named
> pattern (one with optab, as verified by preprocessing insn-opinit.c) which
> does something like that.  Fortunately, the only user of the corresponding
> optab does allow the expander to FAIL and even the pattern itself has a
> FAIL; already, so this patch just moves the condition into the body as
> a condition to FAIL.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-12-05  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/92791
>         * config/i386/i386.md (movstrict<mode>): Move test for
>         TARGET_PARTIAL_REG_STALL and not optimizing for size from
>         expander's condition to the body - FAIL; in that case.

OK for trunk and backports.

Thanks,
Uros.

> --- gcc/config/i386/i386.md.jj  2019-12-03 09:22:17.421777187 +0100
> +++ gcc/config/i386/i386.md     2019-12-04 16:35:34.193669660 +0100
> @@ -2801,10 +2801,11 @@ (define_peephole2
>  (define_expand "movstrict<mode>"
>    [(set (strict_low_part (match_operand:SWI12 0 "register_operand"))
>         (match_operand:SWI12 1 "general_operand"))]
> -  "!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun)"
> +  ""
>  {
>    gcc_assert (SUBREG_P (operands[0]));
> -  if (GET_MODE_CLASS (GET_MODE (SUBREG_REG (operands[0]))) != MODE_INT)
> +  if ((TARGET_PARTIAL_REG_STALL && optimize_function_for_speed_p (cfun))
> +      || GET_MODE_CLASS (GET_MODE (SUBREG_REG (operands[0]))) != MODE_INT)
>      FAIL;
>  })
>
>
>         Jakub
>