On 1/19/25 4:35 AM, Jin Ma wrote:
Although we have handled the vl of XTheadVector correctly in the
expand phase and predicates, the results show that the work is
still insufficient.
In the curr_insn_transform function, the insn is transformed from:
(insn 69 67 225 12 (set (mem:RVVM8SF (reg/f:DI 218 [ _77 ]) [0 S[128, 128]
A32])
(if_then_else:RVVM8SF (unspec:RVVMF4BI [
(const_vector:RVVMF4BI repeat [
(const_int 1 [0x1])
])
(reg:DI 209)
(const_int 0 [0])
(reg:SI 66 vl)
(reg:SI 67 vtype)
] UNSPEC_VPREDICATE)
(reg/v:RVVM8SF 143 [ _xx ])
(mem:RVVM8SF (reg/f:DI 218 [ _77 ]) [0 S[128, 128] A32])))
(expr_list:REG_DEAD (reg/v:RVVM8SF 143 [ _xx ])
(nil)))
to
(insn 69 284 225 11 (set (mem:RVVM8SF (reg/f:DI 18 s2 [orig:218 _77 ] [218]) [0
S[128, 128] A32])
(if_then_else:RVVM8SF (unspec:RVVMF4BI [
(const_vector:RVVMF4BI repeat [
(const_int 1 [0x1])
])
(const_int 1 [0x1])
(const_int 0 [0])
(reg:SI 66 vl)
(reg:SI 67 vtype)
] UNSPEC_VPREDICATE)
(reg/v:RVVM8SF 104 v8 [orig:143 _xx ] [143])
(mem:RVVM8SF (reg/f:DI 18 s2 [orig:218 _77 ] [218]) [0 S[128,
128] A32])))
(nil))
Looking at the log for the reload pass, it is found that "Changing pseudo 209 in
operand 3 of insn 69 on equiv 0x1".
It converts the vl operand in insn from the expected register(reg:DI 209) to the
constant 1(const_int 1 [0x1]).
This conversion occurs because, although the predicate for the vl operand is
restricted by "vector_length_operand" in the pattern, the constraint is still
"rK", which allows the transformation.
As I mentioned before, this is most likely an LRA specific behavior --
in general if the predicate disallows a particular operand value, then
the constraint really doesn't matter. There have been exceptions to
this in the past, particularly in reload and I wouldn't be terribly
surprised if LRA has similar hackery.
Based on the past experience with reload I'd hazard a guess we've got a
REG_EQUIV/REG_EQUAL note that produces a function-wide equivalence and
we're substituting in the value on the assumption that a constraint
failure will trigger reloading the value. And in this case the "K" part
of the constraint says things are OK without reloading and we leave it
as-is.
The first time the affected insn is re-recognized after the substitution
it's going to fail to recognize due to the operand predicate failure.
The issue is that changing the "rK" constraint to "rJ" for the constraint of vl
operand in the pattern would prevent this conversion, But unfortunately this
will
conflict with RVV (RISC-V Vector Extension).
Based on the review's recommendations, the best solution for now is to create
a new constraint to distinguish between RVV and XTheadVector, which is exactly
what this patch does.
PR 116593
gcc/ChangeLog:
* config/riscv/constraints.md (vl): New.
* config/riscv/thead-vector.md: Replacing rK with rvl.
* config/riscv/vector.md: Likewise.
gcc/testsuite/ChangeLog:
* g++.target/riscv/xtheadvector/pr116593.C: New test.
* g++.target/riscv/xtheadvector/xtheadvector.exp: New test.
Reported-by: nihui <shuizhuyuan...@gmail.com>
---
gcc/config/riscv/constraints.md | 6 +
gcc/config/riscv/thead-vector.md | 18 +-
gcc/config/riscv/vector.md | 476 +++++++++---------
.../g++.target/riscv/xtheadvector/pr116593.C | 47 ++
.../riscv/xtheadvector/xtheadvector.exp | 34 ++
5 files changed, 334 insertions(+), 247 deletions(-)
create mode 100644 gcc/testsuite/g++.target/riscv/xtheadvector/pr116593.C
create mode 100644
gcc/testsuite/g++.target/riscv/xtheadvector/xtheadvector.exp
diff --git a/gcc/testsuite/g++.target/riscv/xtheadvector/xtheadvector.exp
b/gcc/testsuite/g++.target/riscv/xtheadvector/xtheadvector.exp
So much like my question on an earlier patch. Would it make more sense
to add the xtheadvector directory to the existing rvv.exp driver rather
than adding another driver and having tests in the xtheadvector
directory behave differently than those in the other directories?
As far as the constraint change itself, that all looks pretty good. So
we just need to settle the testsuite driver change.
jeff
