[committed] RISC-V: Make stack_save_restore tests more robust
Checks
Commit Message
Spurred by Jivan's patch and a desire for cleaner testresults, I went
ahead and make the stack_save_restore tests independent of the precise
stack size by using a regexp.
Pushed to the trunk.
Jeff
commit e1f096a3cc96c71907cfbc7b8baf67a3d863cb6d
Author: Jeff Law <jlaw@ventanamicro.com>
Date: Fri Aug 25 16:34:17 2023 -0600
RISC-V: Make stack_save_restore tests more robust
Spurred by Jivan's patch and a desire for cleaner testresults, I went ahead and
make the stack_save_restore tests independent of the precise stack size by
using a regexp.
gcc/testsuite/
* gcc.target/riscv/stack_save_restore_1.c: Robustify.
* gcc.target/riscv/stack_save_restore_2.c: Robustify.
Comments
On 8/25/23 15:36, Jeff Law wrote:
> Spurred by Jivan's patch and a desire for cleaner testresults, I went
> ahead and make the stack_save_restore tests independent of the precise
> stack size by using a regexp.
>
> Pushed to the trunk.
>
> Jeff
Hi Jeff, A recent change that I'm still bisecting [1] caused
stack_save_restore_2.c to start failing.
Debug log:
Executing on host: /github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/xgcc -B/github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/ /github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c -march=rv32gcv -mabi=ilp32d -mcmodel=medlow -fdiagnostics-plain-output -O0 -march=rv32imafc -mabi=ilp32f -msave-restore -O2 -fno-schedule-insns -fno-schedule-insns2 -fno-unroll-loops -fno-peel-loops -fno-lto -S -o stack_save_restore_2.s (timeout = 600)
spawn -ignore SIGHUP /github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/xgcc -B/github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/ /github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c -march=rv32gcv -mabi=ilp32d -mcmodel=medlow -fdiagnostics-plain-output -O0 -march=rv32imafc -mabi=ilp32f -msave-restore -O2 -fno-schedule-insns -fno-schedule-insns2 -fno-unroll-loops -fno-peel-loops -fno-lto -S -o stack_save_restore_2.s
PASS: gcc.target/riscv/stack_save_restore_2.c -O0 (test for excess errors)
body: \tcall t0,__riscv_save_(3|4)
\taddi sp,sp,-[0-9]+
.*\tli t0,-[0-9]+
\tadd sp,sp,t0
.*\tli t0,[0-9]+
\tadd sp,sp,t0
.*\taddi sp,sp,[0-9]+
\ttail __riscv_restore_(3|4)
against: call t0,__riscv_save_5
addi sp,sp,-2016
fsw fs0,2012(sp)
fsw fs1,2008(sp)
fsw fs2,2004(sp)
fsw fs3,2000(sp)
fsw fs4,1996(sp)
li t0,-12288
add sp,sp,t0
call getf
fmv.s fs1,fa0
call getf
fmv.s fs4,fa0
call getf
fmv.s fs3,fa0
call getf
fmv.s fs2,fa0
li s0,0
fmv.s.x fs0,zero
lui a5,%hi(.LC0)
lw s2,%lo(.LC0)(a5)
lw s3,%lo(.LC0+4)(a5)
addi s4,sp,1984
li s1,4096
addi s1,s1,-528
call my_getchar
call __floatsidf
mv a2,s2
mv a3,s3
call __muldf3
call __truncdfsf2
slli a5,s0,2
add a5,s4,a5
fsw fa0,-1984(a5)
flw fa5,-1984(a5)
fadd.s fs0,fs0,fa5
addi s0,s0,1
bne s0,s1,.L2
fadd.s fa5,fs1,fs0
fadd.s fa5,fa5,fs4
fadd.s fa5,fa5,fs3
fadd.s fa5,fa5,fs2
fcvt.w.s a0,fa5,rtz
li t0,12288
add sp,sp,t0
flw fs0,2012(sp)
flw fs1,2008(sp)
flw fs2,2004(sp)
flw fs3,2000(sp)
flw fs4,1996(sp)
addi sp,sp,2016
tail __riscv_restore_5
FAIL: gcc.target/riscv/stack_save_restore_2.c -O0 check-function-bodies bar
It looks like the issue is that your regex matches
__riscv_save_(3|4) where now gcc emits __riscv_restore_5.
Would it be OK to update the regex to also accept 5 (& are we going to
bump into this again)?
diff --git a/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c
b/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c
index 4c549cb11ae..bc95736cf8e 100644
--- a/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c
+++ b/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c
@@ -7,7 +7,7 @@ float getf();
/*
** bar:
-** call t0,__riscv_save_(3|4)
+** call t0,__riscv_save_(3|4|5)
** addi sp,sp,-[0-9]+
** ...
** li t0,-[0-9]+
@@ -17,7 +17,7 @@ float getf();
** add sp,sp,t0
** ...
** addi sp,sp,[0-9]+
-** tail __riscv_restore_(3|4)
+** tail __riscv_restore_(3|4|5)
*/
int bar()
{
If we're going to run into this again, it might make sense to allow a
wider range of numbers (up to 9):
diff --git a/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c
b/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c
index 4c549cb11ae..1d5b950130e 100644
--- a/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c
+++ b/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c
@@ -7,7 +7,7 @@ float getf();
/*
** bar:
-** call t0,__riscv_save_(3|4)
+** call t0,__riscv_save_([3-9])
** addi sp,sp,-[0-9]+
** ...
** li t0,-[0-9]+
@@ -17,7 +17,7 @@ float getf();
** add sp,sp,t0
** ...
** addi sp,sp,[0-9]+
-** tail __riscv_restore_(3|4)
+** tail __riscv_restore_([3-9])
*/
int bar()
{
Patrick
[1] The commit is in this range:
https://github.com/gcc-mirror/gcc/compare/a4ca8691333344cecc595d1af8b21e51f588e2f2...4d49685d671e4e604b2b873ada65aaac89348794
On 10/27/23 11:34, Patrick O'Neill wrote:
>
> On 8/25/23 15:36, Jeff Law wrote:
>> Spurred by Jivan's patch and a desire for cleaner testresults, I went
>> ahead and make the stack_save_restore tests independent of the precise
>> stack size by using a regexp.
>>
>> Pushed to the trunk.
>>
>> Jeff
>
> Hi Jeff, A recent change that I'm still bisecting [1] caused
> stack_save_restore_2.c to start failing.
>
> Debug log:
>
> Executing on host: /github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/xgcc -B/github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/ /github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c -march=rv32gcv -mabi=ilp32d -mcmodel=medlow -fdiagnostics-plain-output -O0 -march=rv32imafc -mabi=ilp32f -msave-restore -O2 -fno-schedule-insns -fno-schedule-insns2 -fno-unroll-loops -fno-peel-loops -fno-lto -S -o stack_save_restore_2.s (timeout = 600)
> spawn -ignore SIGHUP /github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/xgcc -B/github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/ /github/patrick-postcommit-runner-1/_work/gcc-postcommit-ci/gcc-postcommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.target/riscv/stack_save_restore_2.c -march=rv32gcv -mabi=ilp32d -mcmodel=medlow -fdiagnostics-plain-output -O0 -march=rv32imafc -mabi=ilp32f -msave-restore -O2 -fno-schedule-insns -fno-schedule-insns2 -fno-unroll-loops -fno-peel-loops -fno-lto -S -o stack_save_restore_2.s
> PASS: gcc.target/riscv/stack_save_restore_2.c -O0 (test for excess errors)
> body: \tcall t0,__riscv_save_(3|4)
> \taddi sp,sp,-[0-9]+
> .*\tli t0,-[0-9]+
> \tadd sp,sp,t0
> .*\tli t0,[0-9]+
> \tadd sp,sp,t0
> .*\taddi sp,sp,[0-9]+
> \ttail __riscv_restore_(3|4)
>
> against: call t0,__riscv_save_5
> addi sp,sp,-2016
> fsw fs0,2012(sp)
> fsw fs1,2008(sp)
> fsw fs2,2004(sp)
> fsw fs3,2000(sp)
> fsw fs4,1996(sp)
> li t0,-12288
> add sp,sp,t0
> call getf
> fmv.s fs1,fa0
> call getf
> fmv.s fs4,fa0
> call getf
> fmv.s fs3,fa0
> call getf
> fmv.s fs2,fa0
> li s0,0
> fmv.s.x fs0,zero
> lui a5,%hi(.LC0)
> lw s2,%lo(.LC0)(a5)
> lw s3,%lo(.LC0+4)(a5)
> addi s4,sp,1984
> li s1,4096
> addi s1,s1,-528
> call my_getchar
> call __floatsidf
> mv a2,s2
> mv a3,s3
> call __muldf3
> call __truncdfsf2
> slli a5,s0,2
> add a5,s4,a5
> fsw fa0,-1984(a5)
> flw fa5,-1984(a5)
> fadd.s fs0,fs0,fa5
> addi s0,s0,1
> bne s0,s1,.L2
> fadd.s fa5,fs1,fs0
> fadd.s fa5,fa5,fs4
> fadd.s fa5,fa5,fs3
> fadd.s fa5,fa5,fs2
> fcvt.w.s a0,fa5,rtz
> li t0,12288
> add sp,sp,t0
> flw fs0,2012(sp)
> flw fs1,2008(sp)
> flw fs2,2004(sp)
> flw fs3,2000(sp)
> flw fs4,1996(sp)
> addi sp,sp,2016
> tail __riscv_restore_5
>
> FAIL: gcc.target/riscv/stack_save_restore_2.c -O0 check-function-bodies bar
>
> It looks like the issue is that your regex matches
> __riscv_save_(3|4) where now gcc emits __riscv_restore_5.
>
> Would it be OK to update the regex to also accept 5 (& are we going to
> bump into this again)?
Thanks for looking at this -- my tester flagged them yesterday as well
and I hadn't dug into them yet:
> Tests that now fail, but worked before (11 tests):
>
> gcc.target/riscv/rv32i_zcmp.c -Os check-function-bodies test1
> gcc.target/riscv/rv32i_zcmp.c -Os check-function-bodies test2_step1_0_size
> gcc.target/riscv/rv32i_zcmp.c -Os check-function-bodies test3
> gcc.target/riscv/stack_save_restore_2.c -O0 check-function-bodies bar
> gcc.target/riscv/stack_save_restore_2.c -O1 check-function-bodies bar
> gcc.target/riscv/stack_save_restore_2.c -O2 check-function-bodies bar
> gcc.target/riscv/stack_save_restore_2.c -O2 -flto -fno-use-linker-plugin -flto-partition=none check-function-bodies bar
> gcc.target/riscv/stack_save_restore_2.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects check-function-bodies bar
> gcc.target/riscv/stack_save_restore_2.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions check-function-bodies bar
> gcc.target/riscv/stack_save_restore_2.c -O3 -g check-function-bodies bar
> gcc.target/riscv/stack_save_restore_2.c -Os check-function-bodies bar
Yes, I think accepting more cases here is quite reasonable. In fact,
you might just change it to look for __riscv_save_ without the numeric
suffix.
jeff
@@ -8,15 +8,15 @@ float getf();
/*
** bar:
** call t0,__riscv_save_(3|4)
-** addi sp,sp,-2032
+** addi sp,sp,-[0-9]+
** ...
-** li t0,-12288
+** li t0,-[0-9]+
** add sp,sp,t0
** ...
-** li t0,12288
+** li t0,[0-9]+
** add sp,sp,t0
** ...
-** addi sp,sp,2032
+** addi sp,sp,[0-9]+
** tail __riscv_restore_(3|4)
*/
int bar()
@@ -8,15 +8,15 @@ float getf();
/*
** bar:
** call t0,__riscv_save_(3|4)
-** addi sp,sp,-2032
+** addi sp,sp,-[0-9]+
** ...
-** li t0,-12288
+** li t0,-[0-9]+
** add sp,sp,t0
** ...
-** li t0,12288
+** li t0,[0-9]+
** add sp,sp,t0
** ...
-** addi sp,sp,2032
+** addi sp,sp,[0-9]+
** tail __riscv_restore_(3|4)
*/
int bar()