On 1/22/25 12:29 AM, Robin Dapp wrote:
Hi,
after testing on the BPI (4.2% improvement for x264 input 1, 4.4% for input 2)
and the discussion in PR117173 I figured it's best to disable the two-source
permutes by default for now. We quickly talked about this on the patchwork
call last week. Conclusion was to just post the patch and discuss here.
The patch adds a parameter "riscv-two-source-permutes" which restores
the old behavior. It does not add a uarch knob to override the default.
It's still possible to get two-source permutes in a register via
the expander but the obvious constant cases are covered.
Regtested on rv64gcv_zvl512b.
Regards
Robin
PR target/117173
gcc/ChangeLog:
* config/riscv/riscv-v.cc (shuffle_generic_patterns): Only
support single-source permutes by default.
* config/riscv/riscv.opt: New param "riscv-two-source-permutes".
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/rvv.exp: Run with two-source permutes.
* lib/target-supports.exp: Ditto.
* gcc.dg/fold-perm-2.c: Ditto.
* gcc.dg/pr54346.c: Ditto.
* gcc.dg/vect/costmodel/riscv/rvv/dynamic-lmul4-10.c: Ditto.
* gcc.dg/vect/costmodel/riscv/rvv/dynamic-lmul4-12.c: Ditto.
* gcc.dg/vect/costmodel/riscv/rvv/dynamic-lmul4-6.c: Ditto.
* gcc.dg/vect/costmodel/riscv/rvv/dynamic-lmul4-8.c: Ditto.
* gcc.dg/vect/costmodel/riscv/rvv/dynamic-lmul8-12.c: Ditto.
* gcc.dg/vect/costmodel/riscv/rvv/pr111848.c: Ditto.
* gcc.dg/vect/costmodel/riscv/rvv/rvv-costmodel-vect.exp: Ditto.
So this isn't a regression, but I can also understand the desire to fix
this fairly significant performance issue.
IMHO this is extremely safe -- if rejecting two operand permutes here
were to result in a failure I would argue that we've got a clear bug in
our other permutation support that could be triggered without this change.
So I'm OK with it as well. I think giving folks through the end of the
week to chime in with potential objections would be wise though.
jeff