[PING] [PATCH] Fix PR rtl-optimization/pr60663

[Zhenqiang Chen <zhenqiang dot chen at linaro dot org>]

Ping?

Bootstrap and no make check regression on X86-64.

Bootstrap on ARM. In ARM regression test, some new PASS and FAIL of
debug info check for gcc.dg/guality/pr36728-1.c and
gcc.dg/guality/pr36728-2.c since register allocation result is
different with the patch. There is no real new FAIL due to the patch.

Thanks!
-Zhenqiang


On 27 March 2014 16:44, Zhenqiang Chen <zhenqiang.chen@linaro.org> wrote:
> On 26 March 2014 15:45, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Wed, Mar 26, 2014 at 03:30:44PM +0800, Zhenqiang Chen wrote:
>>> Agree. CSE should never modify asm insns to drop some of the outputs.
>>
>> So the right fix is top prevent this from happening, not papering over about
>> it.
>>>
>>> But in this case, CSE does not drop any of the outputs. It just takes
>>> the SRC of a set and replace the reference of the set. And the
>>> instruction validation tells CSE that it is legal instruction after
>>> replacement. (The original correct asm insn is optimized away after
>>> this replacement)
>>>
>>> I think it is common for most rtl-optimizations to do such kind of
>>> validation. So to avoid such kind of bug, check_asm_operands must tell
>>> the optimizer the asm is illegal.
>>
>> As it is wrong if CSE does that even with asm ("" : "=r" (i), "=r" (j));,
>> your patch is not the right place to fix this.  CSE just must check where
>> the ASM_OPERANDS is coming from and if it comes from a PARALLEL with
>> multiple outputs, either give up or duplicate all outputs (if it makes sense
>> at all).  Or just don't enter into the hash tables ASM_OPERANDS with
>> multiple outputs.
>
> Patch is updated:
>
> diff --git a/gcc/cse.c b/gcc/cse.c
> index 852d13e..ce84982 100644
> --- a/gcc/cse.c
> +++ b/gcc/cse.c
> @@ -4280,6 +4280,19 @@ find_sets_in_insn (rtx insn, struct set **psets)
>                 ;
>               else if (GET_CODE (SET_SRC (y)) == CALL)
>                 ;
> +             else if (GET_CODE (SET_SRC (y)) == ASM_OPERANDS)
> +               {
> +                 if (i + 1 < lim)
> +                   {
> +                     rtx n = XVECEXP (x, 0, i + 1);
> +                     /* For inline assemble with multiple outputs, we can not
> +                        handle the SET separately.  Refer PR60663.  */
> +                     if (GET_CODE (n) == SET
> +                         && GET_CODE (SET_SRC (n)) == ASM_OPERANDS)
> +                       break;
> +                   }
> +                 sets[n_sets++].rtl = y;
> +               }
>               else
>                 sets[n_sets++].rtl = y;
>             }
>
> Thanks!
> -Zhenqiang