On Thu, Dec 5, 2019 at 9:21 AM Jakub Jelinek <ja...@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 <ja...@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
>
