Hello!

> PR 58079 is about the do_SUBST assert:
>
>       /* Sanity check that we're replacing oldval with a CONST_INT
> that is a valid sign-extension for the original mode.  */
>       gcc_assert (INTVAL (newval)
>  == trunc_int_for_mode (INTVAL (newval), GET_MODE (oldval)));
>
> triggering while trying to optimise the temporary result:
>
>   (eq (const_int -73 [0xffffffffffffffb7])
>       (reg:QI 234 [ MEM[(const u8 *)buf_7(D) + 4B] ])
>
> combine_simplify_rtx calls simplify_comparison, which first canonicalises
> the order so that the constant is second and then promotes the comparison
> to SImode (the first supported comparison mode on MIPS).  So we end up with:
>
>   (eq (zero_extend:SI (reg:QI 234 [ MEM[(const u8 *)buf_7(D) + 4B] ]))
>       (const_int 183 [0xb7]))
>
> So far so good.  But combine_simplify_rtx then tries to install the
> new operands in-place, with the second part of:
>
>  /* If the code changed, return a whole new comparison.  */
>  if (new_code != code)
>    return gen_rtx_fmt_ee (new_code, mode, op0, op1);
>
>  /* Otherwise, keep this operation, but maybe change its operands.
>     This also converts (ne (compare FOO BAR) 0) to (ne FOO BAR).  */
>  SUBST (XEXP (x, 0), op0);
>  SUBST (XEXP (x, 1), op1);
>
> And this triggers the assert because we're replacing a QImode register
> with the non-QImode constant 183.
>
> One fix would be to extend the new_code != code condition, as below.
> This should avoid the problem cases without generating too much
> garbage rtl.  But if the condition's getting this complicated,
> perhaps it'd be better just to avoid SUBST here?  I.e.
>
>  if (new_code != code || op0 != XEXP (x, 0) || op1 != XEXP (x, 1))
>    return gen_rtx_fmt_ee (new_code, mode, op0, op1);
>
> Just asking though. :-)
>
> Tested on mipsisa64-elf.  OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
> PR rtl-optimization/58079
> * combine.c (combine_simplify_rtx): Avoid using SUBST if
> simplify_comparison has widened a comparison with an integer.
>
> gcc/testsuite/
> * gcc.dg/torture/pr58079.c: New test.

I would like to backport this patch to 4.8 branch. The patch fixes the
failure on alpha-linux-gnu and also (reportedly) fixes the same
failure on mips and pa targets.

I have bootstrapped and regression test the patch on
x86_64-pc-linux-gnu {,-m32} and started bootstrap and regtests on
alphaev68-linux-gnu on 4.8 branch for all default languages + obj-c++
and go. The latter will take some ~10 hours.

OK to backort the patch to the 4.8 branch if both tests pass?

Thanks,
Uros.

Reply via email to