Re: [PATCH] Fix PR rtl-optimization/pr60663

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

On 26 March 2014 15:00, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Mar 26, 2014 at 02:16:16PM +0800, Zhenqiang Chen wrote:
>> The patch checks the number of the expected operands in
>> ASM_OPERANDS_TEMPLATE with the same logic as it in output_asm_insn to
>> make sure the ASM_OPERANDS are legal.
>>
>> Bootstrap and no make check regression on X86-64 and ARM chromebook.
>>
>> OK for trunk?
>
> No, this is very wrong.  How many operands you refer to and how many times

This is how the output_asm_insn (final.c) check and output the error
message. The logic in my patch is the same as it in output_asm_insn.

> to each is completely unrelated to the fact that CSE should never modify
> asm insns to drop some of the outputs.

Agree. CSE should never modify asm insns to drop some of the outputs.

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.

> You can refer to no operands at all and still it would be invalid to drop
> the outputs, or you can have output template like "%0 %0 %0 %0 %0 ... million times %0".

I think it does not check the number of %0, but the max "n" in %n.

Thanks!
-Zhenqiang

>         Jakub