VSETVL PASS is supposed to insert "vsetvli" into optimal place so "vsetvli" inserted by VSETVL PASS shouldn't involved into instruction scheduling.
juzhe.zh...@rivai.ai From: Jeff Law Date: 2025-02-13 08:47 To: Edwin Lu; gcc-patches CC: gnu-toolchain; vineetg; juzhe.zhong Subject: Re: [PATCH] RISC-V: Prevent speculative vsetvl insn scheduling On 2/12/25 4:27 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 (riscv_sched_can_speculate_insn): > (TARGET_SCHED_CAN_SPECULATE_INSN): Implement. Correct me if I'm wrong, there's not anything inherently wrong with the speculation from a correctness standpoint. This is "just" a performance issue, right? And from a performance standpoint speculation of the vsetvl could vary pretty wildly based on uarch characteristics. I can easily see cases where it it wildly bad, wildly good and don't really care. Point being it seems like it should be controlled by a uarch setting rather than always avoiding or always enabling. Other thoughts? Jeff