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

[Jeff Law]

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