RFC/A: PR 58079: CONST_INT mode assert in combine.c:do_SUBST

[Uros Bizjak]

On Mon, Oct 28, 2013 at 5:28 PM, Uros Bizjak <ubizjak@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.