On 2/13/25 3:54 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.
Since this is purely a performance related patch, gate the target hook
with an opt flag to see the fallout.
PR 117974
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_sched_can_speculate_insn):
(TARGET_SCHED_CAN_SPECULATE_INSN): Implement.
* config/riscv/riscv.opt: Add temporary opt.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/vsetvl/pr117974.c: New test.
I was kind of thinking we'd have a flag in the tuning structure so that
each uarch could make a reasonable default selection rather than having
it controlled by a flag.
I realize there may not be any designs upstream which would set this
right now, but it puts the right infrastructure in place. Very similar
to the divmod expansion stuff from a while back.
Q. Does Rivos have a cpu defined in your internal tree? We do that for
ours as keep it as a local change for now. Assuming you do, then adding
the bits to the tuning structure should make it trivial to make this
behavior the default on your design internally.
Jeff
