On Mon, Oct 28, 2013 at 5:28 PM, Uros Bizjak <ubiz...@gmail.com> wrote:
>> 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? The regression test was OK in both cases, so I went ahead and installed the patch. The fix is kind of obvious anyway. Uros.