On 5/21/25 11:07, Jeff Law wrote: > On 5/20/25 4:05 PM, Edwin Lu wrote: >> The instruction scheduler appears to be speculatively hoisting vsetvl >> insns outside of their basic block without checking for data >> dependencies. This resulted in a situation where the following occurs >> >> vsetvli a5,a1,e32,m1,tu,ma >> vle32.v v2,0(a0) >> sub a1,a1,a5 <-- a1 potentially set to 0 >> sh2add a0,a5,a0 >> vfmacc.vv v1,v2,v2 >> vsetvli a5,a1,e32,m1,tu,ma <-- incompatible vinfo. update vl to 0 >> beq a1,zero,.L12 <-- check if avl is 0 >> >> This patch would essentially delay the vsetvl update to after the branch >> to prevent unnecessarily updating the vinfo at the end of a basic block. >> >> PR 117974 >> >> gcc/ChangeLog: >> >> * config/riscv/riscv.cc (struct riscv_tune_param): Add tune >> param. >> (riscv_sched_can_speculate_insn): Implement. >> (TARGET_SCHED_CAN_SPECULATE_INSN): Implement. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/rvv/vsetvl/pr117974.c: New test. > Though note it failed CI on the new test: > >> FAIL: gcc.target/riscv/rvv/vsetvl/pr117974.c -O3 -fomit-frame-pointer >> -funroll-loops -fpeel-loops -ftracer -finline-functions >> scan-assembler-times beq\\s+[a-x0-9]+,zero,.L12\\s+vsetvli 3 > So something probably needs a minor adjustment.
So as of tip of tree, even w/o the patch we no longer see the pattern "beq <reg>, 0, .L12 " They are replaced with reg anyways, so the test won't even show issue, even w/o the fix. beq a1,a5,.L12 beq a6,t0,.L12 beq a7,t3,.L12 I think a better proxy of improvement would be to grep for vsetvls with null output VL. orig vsetvli a5,a1,e8,mf4,ta,ma vsetvli t0,a6,e8,mf4,ta,ma vsetvli zero,a5,e32,m1,tu,ma vsetvli zero,t0,e32,m1,ta,ma vsetvli t3,a7,e8,mf4,ta,ma vsetvli t4,t1,e8,mf4,ta,ma vsetvli zero,t0,e32,m1,tu,ma vsetvli zero,t3,e32,m1,tu,ma vsetvli zero,t4,e32,m1,tu,ma patch vsetvli a5,a1,e32,m1,tu,ma vsetvli t1,t0,e32,m1,tu,ma vsetvli a2,a1,e32,m1,tu,ma vsetvli t4,t3,e32,m1,tu,ma -Vineet
