This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PING] [PATCH] Fix PR rtl-optimization/pr60663
- From: Zhenqiang Chen <zhenqiang dot chen at linaro dot org>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 1 Apr 2014 11:41:12 +0800
- Subject: [PING] [PATCH] Fix PR rtl-optimization/pr60663
- Authentication-results: sourceware.org; auth=none
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