diff mbox

Resolve pr44194-1.c failure by refining scan-rtl-dump-not pattern

Message ID 52CFC98B.3090507@arm.com
State New
Headers show

Commit Message

Renlin Li Jan. 10, 2014, 10:20 a.m. UTC
Hi all,

This patch will resolve testsuite/gcc.dg/pr44194-1.c failure observed in 
aarch64 target by refining the second scan-rtl-dump-not pattern.

For aarch64 target, the following barrier is generated which will 
exactly match the current pattern
"insn \[^\n\]*set \\(mem". This is undesired. By adjusting the pattern, 
this scenario is excluded.

(insn 29 28 30 (set (mem:BLK (scratch) [0  A8])
         (unspec:BLK [
                 (reg/f:DI 31 sp)
                 (reg/f:DI 29 x29)
             ] UNSPEC_PRLG_STK)) pr44194-1.c:10 704 {stack_tie}
      (nil))

Okay for trunk?

Regards,
Renlin Li

Comments

Eric Botcazou Jan. 10, 2014, 10:42 a.m. UTC | #1
> This patch will resolve testsuite/gcc.dg/pr44194-1.c failure observed in
> aarch64 target by refining the second scan-rtl-dump-not pattern.
> 
> For aarch64 target, the following barrier is generated which will
> exactly match the current pattern
> "insn \[^\n\]*set \\(mem". This is undesired. By adjusting the pattern,
> this scenario is excluded.
> 
> (insn 29 28 30 (set (mem:BLK (scratch) [0  A8])
>          (unspec:BLK [
>                  (reg/f:DI 31 sp)
>                  (reg/f:DI 29 x29)
>              ] UNSPEC_PRLG_STK)) pr44194-1.c:10 704 {stack_tie}
>       (nil))

Please use "scratch" instead of "stack_tie" as marker, it's more general.
Renlin Li Jan. 10, 2014, 11:28 a.m. UTC | #2
Hi Eric,

Thank you for your suggestion!
I have made the adjustment and test it. Please check the new attachment.

Regards,
Renlin Li

On 10/01/14 10:42, Eric Botcazou wrote:
>> This patch will resolve testsuite/gcc.dg/pr44194-1.c failure observed in
>> aarch64 target by refining the second scan-rtl-dump-not pattern.
>>
>> For aarch64 target, the following barrier is generated which will
>> exactly match the current pattern
>> "insn \[^\n\]*set \\(mem". This is undesired. By adjusting the pattern,
>> this scenario is excluded.
>>
>> (insn 29 28 30 (set (mem:BLK (scratch) [0  A8])
>>           (unspec:BLK [
>>                   (reg/f:DI 31 sp)
>>                   (reg/f:DI 29 x29)
>>               ] UNSPEC_PRLG_STK)) pr44194-1.c:10 704 {stack_tie}
>>        (nil))
> Please use "scratch" instead of "stack_tie" as marker, it's more general.
>
Renlin Li Jan. 19, 2014, 7:43 p.m. UTC | #3
On 10/01/14 11:28, Renlin Li wrote:
> Hi Eric,
>
> Thank you for your suggestion!
> I have made the adjustment and test it. Please check the new attachment.
>
> Regards,
> Renlin Li
>
> On 10/01/14 10:42, Eric Botcazou wrote:
>>> This patch will resolve testsuite/gcc.dg/pr44194-1.c failure 
>>> observed in
>>> aarch64 target by refining the second scan-rtl-dump-not pattern.
>>>
>>> For aarch64 target, the following barrier is generated which will
>>> exactly match the current pattern
>>> "insn \[^\n\]*set \\(mem". This is undesired. By adjusting the pattern,
>>> this scenario is excluded.
>>>
>>> (insn 29 28 30 (set (mem:BLK (scratch) [0  A8])
>>>           (unspec:BLK [
>>>                   (reg/f:DI 31 sp)
>>>                   (reg/f:DI 29 x29)
>>>               ] UNSPEC_PRLG_STK)) pr44194-1.c:10 704 {stack_tie}
>>>        (nil))
>> Please use "scratch" instead of "stack_tie" as marker, it's more 
>> general.
>>
>
Hi,

Any comments?

Kind regards,
Renlin Li
Eric Botcazou Jan. 20, 2014, 9:11 a.m. UTC | #4
> I have made the adjustment and test it. Please check the new attachment.

Thanks, applied on the mainline as obvious.
Richard Sandiford Jan. 21, 2014, 8:37 p.m. UTC | #5
Eric Botcazou <ebotcazou@adacore.com> writes:
>> I have made the adjustment and test it. Please check the new attachment.
>
> Thanks, applied on the mainline as obvious.

It looks like the committed version removed the space after "insn".
Was that intentional?  It now triggers on MIPS because of the frame-related
(insn/f) prologue instruction that stores the link register.  The posted
version works for me FWIW.

And I guess this shows that it'd be worth having a comment saying _why_
the space is needed. :-)

Thanks,
Richard
Eric Botcazou Jan. 21, 2014, 11:34 p.m. UTC | #6
> It looks like the committed version removed the space after "insn".
> Was that intentional?  It now triggers on MIPS because of the frame-related
> (insn/f) prologue instruction that stores the link register.  The posted
> version works for me FWIW.

I found out that it didn't fail with the space on x86 (hence false negative) 
because of the TI marker added by some RTL pass.  Feel free to adjust back.
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/pr44194-1.c b/gcc/testsuite/gcc.dg/pr44194-1.c
index 0182f75..8899500 100644
--- a/gcc/testsuite/gcc.dg/pr44194-1.c
+++ b/gcc/testsuite/gcc.dg/pr44194-1.c
@@ -15,5 +15,5 @@  void func() {
 /* { dg-final { scan-rtl-dump "global deletions = (2|3)" "dse1" } } */
 /* { dg-final { cleanup-rtl-dump "dse1" } } */
 
-/* { dg-final { scan-rtl-dump-not "insn \[^\n\]*set \\(mem" "final" } } */
+/* { dg-final { scan-rtl-dump-not "insn \[^\n\]*set \\(mem(?!.*?stack_tie)" "final" } } */
 /* { dg-final { cleanup-rtl-dump "final" } } */