Re: [committed] RISC-V: Make stack_save_restore tests more robust

[Jeff Law <jeffreyalaw@gmail.com>]

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