* [committed] Remove compromised sh test
@ 2024-06-26 13:22 Jeff Law
2024-06-26 22:12 ` Oleg Endo
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2024-06-26 13:22 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 480 bytes --]
Surya's recent patch to IRA improves the code for sh/pr54602-1.c
slightly. Specifically it's able to eliminate a save/restore in the
prologue/epilogue and a bit of register shuffling.
As a result there literally aren't any insns that can be used to fill
the delay slot of the return, so a nop gets emitted and the test fails.
Given there literally aren't any insns to move into the delay slot, the
best course of action is to just drop the test.
Pushed to the trunk.
Jeff
[-- Attachment #2: P --]
[-- Type: text/plain, Size: 1375 bytes --]
commit 47b68cda2c4afe32e84c5f18da0196c39e5e0edf
Author: Jeff Law <jlaw@ventanamicro.com>
Date: Wed Jun 26 07:20:29 2024 -0600
[committed] Remove compromised sh test
Surya's recent patch to IRA improves the code for sh/pr54602-1.c slightly.
Specifically it's able to eliminate a save/restore in the prologue/epilogue and
a bit of register shuffling.
As a result there literally aren't any insns that can be used to fill the delay
slot of the return, so a nop gets emitted and the test fails.
Given there literally aren't any insns to move into the delay slot, the best
course of action is to just drop the test.
gcc/testsuite
* gcc.target/sh/pr54602-1.c: Delete test.
diff --git a/gcc/testsuite/gcc.target/sh/pr54602-1.c b/gcc/testsuite/gcc.target/sh/pr54602-1.c
deleted file mode 100644
index e7fb2a9a642..00000000000
--- a/gcc/testsuite/gcc.target/sh/pr54602-1.c
+++ /dev/null
@@ -1,14 +0,0 @@
-/* Verify that the delay slot is stuffed with register pop insns for normal
- (i.e. not interrupt handler) function returns. If everything goes as
- expected we won't see any nop insns. */
-/* { dg-do compile } */
-/* { dg-options "-O1" } */
-/* { dg-final { scan-assembler-not "nop" } } */
-
-int test00 (int a, int b);
-
-int
-test01 (int a, int b, int c, int d)
-{
- return test00 (a, b) + c;
-}
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [committed] Remove compromised sh test
2024-06-26 13:22 [committed] Remove compromised sh test Jeff Law
@ 2024-06-26 22:12 ` Oleg Endo
2024-06-26 22:39 ` Jeff Law
0 siblings, 1 reply; 8+ messages in thread
From: Oleg Endo @ 2024-06-26 22:12 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On Wed, 2024-06-26 at 07:22 -0600, Jeff Law wrote:
> Surya's recent patch to IRA improves the code for sh/pr54602-1.c
> slightly. Specifically it's able to eliminate a save/restore in the
> prologue/epilogue and a bit of register shuffling.
>
> As a result there literally aren't any insns that can be used to fill
> the delay slot of the return, so a nop gets emitted and the test fails.
>
> Given there literally aren't any insns to move into the delay slot, the
> best course of action is to just drop the test.
>
> Pushed to the trunk.
>
> Jeff
I can't reproduce what you are saying.
Which triplet and flags is your test setup using?
For this test case, GCC 13 with -m4 -ml -O1 -fno-pic:
_test01:
mov.l r8,@-r15
sts.l pr,@-r15
mov.l .L3,r0
jsr @r0
mov r6,r8
add r8,r0
lds.l @r15+,pr
rts
mov.l @r15+,r8
.L3:
.long _test00
current GCC master branch with -m4 -ml -O1 -fno-pic:
_test00:
mov.l r8,@-r15
sts.l pr,@-r15
mov.l .L3,r0
jsr @r0
mov r6,r8
add r8,r0
lds.l @r15+,pr
rts
mov.l @r15+,r8
.L4:
.align 2
.L3:
.long _test01
Best regards,
Oleg Endo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [committed] Remove compromised sh test
2024-06-26 22:12 ` Oleg Endo
@ 2024-06-26 22:39 ` Jeff Law
2024-06-26 22:44 ` Oleg Endo
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2024-06-26 22:39 UTC (permalink / raw)
To: Oleg Endo, gcc-patches
On 6/26/24 4:12 PM, Oleg Endo wrote:
>
>
> On Wed, 2024-06-26 at 07:22 -0600, Jeff Law wrote:
>> Surya's recent patch to IRA improves the code for sh/pr54602-1.c
>> slightly. Specifically it's able to eliminate a save/restore in the
>> prologue/epilogue and a bit of register shuffling.
>>
>> As a result there literally aren't any insns that can be used to fill
>> the delay slot of the return, so a nop gets emitted and the test fails.
>>
>> Given there literally aren't any insns to move into the delay slot, the
>> best course of action is to just drop the test.
>>
>> Pushed to the trunk.
>>
>> Jeff
>
> I can't reproduce what you are saying.
> Which triplet and flags is your test setup using?
>
> For this test case, GCC 13 with -m4 -ml -O1 -fno-pic:
No -m flags at all. As plain of a testrun as you can do.
jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [committed] Remove compromised sh test
2024-06-26 22:39 ` Jeff Law
@ 2024-06-26 22:44 ` Oleg Endo
2024-06-27 0:30 ` Jeff Law
0 siblings, 1 reply; 8+ messages in thread
From: Oleg Endo @ 2024-06-26 22:44 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On Wed, 2024-06-26 at 16:39 -0600, Jeff Law wrote:
>
> On 6/26/24 4:12 PM, Oleg Endo wrote:
> >
> >
> > On Wed, 2024-06-26 at 07:22 -0600, Jeff Law wrote:
> > > Surya's recent patch to IRA improves the code for sh/pr54602-1.c
> > > slightly. Specifically it's able to eliminate a save/restore in the
> > > prologue/epilogue and a bit of register shuffling.
> > >
> > > As a result there literally aren't any insns that can be used to fill
> > > the delay slot of the return, so a nop gets emitted and the test fails.
> > >
> > > Given there literally aren't any insns to move into the delay slot, the
> > > best course of action is to just drop the test.
> > >
> > > Pushed to the trunk.
> > >
> > > Jeff
> >
> > I can't reproduce what you are saying.
> > Which triplet and flags is your test setup using?
> >
> > For this test case, GCC 13 with -m4 -ml -O1 -fno-pic:
> No -m flags at all. As plain of a testrun as you can do.
>
OK, then what's the default config of your test setup / triplet?
Can you please show the generated code that you get? Because - like I said
- I can't reproduce it.
Best regards,
Oleg Endo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [committed] Remove compromised sh test
2024-06-26 22:44 ` Oleg Endo
@ 2024-06-27 0:30 ` Jeff Law
2024-06-27 1:27 ` Oleg Endo
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2024-06-27 0:30 UTC (permalink / raw)
To: Oleg Endo, gcc-patches
On 6/26/24 4:44 PM, Oleg Endo wrote:
>
>
> On Wed, 2024-06-26 at 16:39 -0600, Jeff Law wrote:
>>
>> On 6/26/24 4:12 PM, Oleg Endo wrote:
>>>
>>>
>>> On Wed, 2024-06-26 at 07:22 -0600, Jeff Law wrote:
>>>> Surya's recent patch to IRA improves the code for sh/pr54602-1.c
>>>> slightly. Specifically it's able to eliminate a save/restore in the
>>>> prologue/epilogue and a bit of register shuffling.
>>>>
>>>> As a result there literally aren't any insns that can be used to fill
>>>> the delay slot of the return, so a nop gets emitted and the test fails.
>>>>
>>>> Given there literally aren't any insns to move into the delay slot, the
>>>> best course of action is to just drop the test.
>>>>
>>>> Pushed to the trunk.
>>>>
>>>> Jeff
>>>
>>> I can't reproduce what you are saying.
>>> Which triplet and flags is your test setup using?
>>>
>>> For this test case, GCC 13 with -m4 -ml -O1 -fno-pic:
>> No -m flags at all. As plain of a testrun as you can do.
>>
>
> OK, then what's the default config of your test setup / triplet?
> Can you please show the generated code that you get? Because - like I said
> - I can't reproduce it.
test01:
sts.l pr,@-r15 ! 31 [c=4 l=2] movsi_i/10
add #-4,r15 ! 32 [c=4 l=2] *addsi3/0
mov.l .L3,r0 ! 26 [c=10 l=2] movsi_i/0
jsr @r0 ! 12 [c=5 l=2] call_valuei
mov.l r6,@r15 ! 4 [c=4 l=2] movsi_i/8
mov.l @r15,r1 ! 29 [c=1 l=2] movsi_i/5
add r1,r0 ! 30 [c=4 l=2] *addsi3/0
add #4,r15 ! 36 [c=4 l=2] *addsi3/0
lds.l @r15+,pr ! 38 [c=1 l=2] movsi_i/14
rts
nop ! 40 [c=0 l=4] *return_i
Note that there's a scheduling barrier in the RTL between insns 30 and
36. So instructions prior to insn 36 can't be used to fill the delay slot.
jeff
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [committed] Remove compromised sh test
2024-06-27 0:30 ` Jeff Law
@ 2024-06-27 1:27 ` Oleg Endo
2024-06-27 1:54 ` Jeff Law
0 siblings, 1 reply; 8+ messages in thread
From: Oleg Endo @ 2024-06-27 1:27 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On Wed, 2024-06-26 at 18:30 -0600, Jeff Law wrote:
> > >
> >
> > OK, then what's the default config of your test setup / triplet?
> > Can you please show the generated code that you get? Because - like I said
> > - I can't reproduce it.
> test01:
> sts.l pr,@-r15 ! 31 [c=4 l=2] movsi_i/10
> add #-4,r15 ! 32 [c=4 l=2] *addsi3/0
> mov.l .L3,r0 ! 26 [c=10 l=2] movsi_i/0
> jsr @r0 ! 12 [c=5 l=2] call_valuei
> mov.l r6,@r15 ! 4 [c=4 l=2] movsi_i/8
> mov.l @r15,r1 ! 29 [c=1 l=2] movsi_i/5
> add r1,r0 ! 30 [c=4 l=2] *addsi3/0
> add #4,r15 ! 36 [c=4 l=2] *addsi3/0
> lds.l @r15+,pr ! 38 [c=1 l=2] movsi_i/14
> rts
> nop ! 40 [c=0 l=4] *return_i
>
>
> Note that there's a scheduling barrier in the RTL between insns 30 and
> 36. So instructions prior to insn 36 can't be used to fill the delay slot.
>
Thanks. Now I'm also seeing the same result. Needed to specify -O2 to get
that. -O1 was not enough it seems.
I don't know why you said that the code for this case improved -- it has
not?!
I think the test is still valid. The reason for the failure might be
different from the original one (the scheduling barrier for whatever
reason), but the end result is the same -- the last delay slot is not
stuffed, although the 'add r1,r0' could go in there.
I'd like to revert the removal of this test case, as it catches a valid
issue.
Best regards,
Oleg Endo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [committed] Remove compromised sh test
2024-06-27 1:27 ` Oleg Endo
@ 2024-06-27 1:54 ` Jeff Law
0 siblings, 0 replies; 8+ messages in thread
From: Jeff Law @ 2024-06-27 1:54 UTC (permalink / raw)
To: Oleg Endo, gcc-patches
On 6/26/24 7:27 PM, Oleg Endo wrote:
> On Wed, 2024-06-26 at 18:30 -0600, Jeff Law wrote:
>>>>
>>>
>>> OK, then what's the default config of your test setup / triplet?
>>> Can you please show the generated code that you get? Because - like I said
>>> - I can't reproduce it.
>> test01:
>> sts.l pr,@-r15 ! 31 [c=4 l=2] movsi_i/10
>> add #-4,r15 ! 32 [c=4 l=2] *addsi3/0
>> mov.l .L3,r0 ! 26 [c=10 l=2] movsi_i/0
>> jsr @r0 ! 12 [c=5 l=2] call_valuei
>> mov.l r6,@r15 ! 4 [c=4 l=2] movsi_i/8
>> mov.l @r15,r1 ! 29 [c=1 l=2] movsi_i/5
>> add r1,r0 ! 30 [c=4 l=2] *addsi3/0
>> add #4,r15 ! 36 [c=4 l=2] *addsi3/0
>> lds.l @r15+,pr ! 38 [c=1 l=2] movsi_i/14
>> rts
>> nop ! 40 [c=0 l=4] *return_i
>>
>>
>> Note that there's a scheduling barrier in the RTL between insns 30 and
>> 36. So instructions prior to insn 36 can't be used to fill the delay slot.
>>
>
> Thanks. Now I'm also seeing the same result. Needed to specify -O2 to get
> that. -O1 was not enough it seems.
>
> I don't know why you said that the code for this case improved -- it has
> not?!
>
> I think the test is still valid. The reason for the failure might be
> different from the original one (the scheduling barrier for whatever
> reason), but the end result is the same -- the last delay slot is not
> stuffed, although the 'add r1,r0' could go in there.
>
> I'd like to revert the removal of this test case, as it catches a valid
> issue.
Before the IRA patch there is an additional prologue/epilogue
save/restore for a callee saved register. That's what filled the delay
slot before.
THe add r1,r0 can not move down to fill the delay slot. There's
scheduling barrier in the RTL.
Feel free to restore it, but you're just adding a bogus, failing, test
to the testsuite.
jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* [committed] Remove compromised sh test
@ 2024-06-26 13:21 Jeff Law
0 siblings, 0 replies; 8+ messages in thread
From: Jeff Law @ 2024-06-26 13:21 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 480 bytes --]
Surya's recent patch to IRA improves the code for sh/pr54602-1.c
slightly. Specifically it's able to eliminate a save/restore in the
prologue/epilogue and a bit of register shuffling.
As a result there literally aren't any insns that can be used to fill
the delay slot of the return, so a nop gets emitted and the test fails.
Given there literally aren't any insns to move into the delay slot, the
best course of action is to just drop the test.
Pushed to the trunk.
Jeff
[-- Attachment #2: P --]
[-- Type: text/plain, Size: 1375 bytes --]
commit 47b68cda2c4afe32e84c5f18da0196c39e5e0edf
Author: Jeff Law <jlaw@ventanamicro.com>
Date: Wed Jun 26 07:20:29 2024 -0600
[committed] Remove compromised sh test
Surya's recent patch to IRA improves the code for sh/pr54602-1.c slightly.
Specifically it's able to eliminate a save/restore in the prologue/epilogue and
a bit of register shuffling.
As a result there literally aren't any insns that can be used to fill the delay
slot of the return, so a nop gets emitted and the test fails.
Given there literally aren't any insns to move into the delay slot, the best
course of action is to just drop the test.
gcc/testsuite
* gcc.target/sh/pr54602-1.c: Delete test.
diff --git a/gcc/testsuite/gcc.target/sh/pr54602-1.c b/gcc/testsuite/gcc.target/sh/pr54602-1.c
deleted file mode 100644
index e7fb2a9a642..00000000000
--- a/gcc/testsuite/gcc.target/sh/pr54602-1.c
+++ /dev/null
@@ -1,14 +0,0 @@
-/* Verify that the delay slot is stuffed with register pop insns for normal
- (i.e. not interrupt handler) function returns. If everything goes as
- expected we won't see any nop insns. */
-/* { dg-do compile } */
-/* { dg-options "-O1" } */
-/* { dg-final { scan-assembler-not "nop" } } */
-
-int test00 (int a, int b);
-
-int
-test01 (int a, int b, int c, int d)
-{
- return test00 (a, b) + c;
-}
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-06-27 1:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-26 13:22 [committed] Remove compromised sh test Jeff Law
2024-06-26 22:12 ` Oleg Endo
2024-06-26 22:39 ` Jeff Law
2024-06-26 22:44 ` Oleg Endo
2024-06-27 0:30 ` Jeff Law
2024-06-27 1:27 ` Oleg Endo
2024-06-27 1:54 ` Jeff Law
-- strict thread matches above, loose matches on Subject: below --
2024-06-26 13:21 Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).