Ignore this. I didn't realize I had the AI review stuff in my reply
window at the time.
On 3/31/2026 10:06 AM, Jeffrey Law wrote:
Main thing that looks wrong:
- `widening_valid_reg_group_p` is checking the **mode alignment**, not
the actual chosen register number / overlap legality.
- It takes only `operands[0]` and does:
- `riscv_get_v_regno_alignment (GET_MODE (op_0)) == 2`
- But that tells you the natural alignment requirement for the
destination mode, not whether the selected hard reg for operand 0/3
actually satisfies the intended overlap pattern.
- For `wo21`/`wo22`, legality depends on the concrete register numbers,
e.g. dest even and src odd for the m2/vf2 overlap case. The predicate as
written does not inspect either operand regno.
- So the new `enabled` test appears ineffective / conceptually wrong.
Related issues:
- The helper name and comment say “predicate valid overlap or not”, but
it only handles `REG_GROUP_OVERLAP_WO21` by checking destination mode
alignment. That does not match the semantics of the new constraints.
- `wo22` is added but never represented in `reg_group_overlap` attribute
values. The attr only has `"none,wo21"`. That may be intentional if the
attr encodes the pattern class rather than every operand constraint, but
it is confusing and worth clarifying.
- The new alternatives:
- dest: `wo22`
- src: `wo21`
- merge: `vu` / `0`
These already seem to encode the overlap relationship structurally for
the m2 case. If so, the new `enabled` attribute logic may be redundant.
If not, it is insufficient.
- The predicate signature probably needs both dest and src RTXes, or
just the insn pattern operands, and should check actual hard-reg numbers
after allocation/reload context if this is meant to validate overlap.
Less important, but I’d ask about:
- The changelog says “Add reg constraint wo21 and wo22” and “Add 2 new
alternative for m2 reg overlap for v[sz]ext.vf2”, but the pattern is
generic. Are these alternatives accidentally offered for modes beyond
LMUL=2? If so, the constraints alone may not restrict to only m2 in the
intended way.
- Wording/docs in `constraints.md` are pretty rough; not a blocker, but
the descriptions are hard to parse.
So I would not take this as-is. The core validation logic seems
incorrect because it does not test the actual register overlap condition.
On 2/26/2026 12:35 AM, [email protected] wrote:
From: Pan Li <[email protected]>
According to the RVV 1.0 spec, the widening ops like vzext.vf2
could have the source registers overlap on constraint, aka:
`
The destination EEW is greater than the source EEW, the source
EMUL is at least 1, and the overlap is in the highest-
numbered part of the destination register group.
`
This PATCH could like to re-enable it start from vzext.vf2 for
lmul m2.
gcc/ChangeLog:
* config/riscv/constraints.md (TARGET_VECTOR ? V_REGS : NO_REGS):
Add reg constraint wo21 and wo22.
* config/riscv/riscv-protos.h (widening_valid_reg_group_p): Add
new func decl to predicate valid overlap or not.
* config/riscv/riscv-v.cc (widening_valid_reg_group_p): Add new
func impl to predicate valid overlap or not.
* config/riscv/riscv.md (none,wo21): Add new attr for reg
overlap insns.
* config/riscv/vector.md: Add 2 new alternative for m2 reg
overlap for v[sz]ext.vf2.
Signed-off-by: Pan Li <[email protected]>
---
gcc/config/riscv/constraints.md | 34 +++++++++++++++++++++++++++++++++
gcc/config/riscv/riscv-protos.h | 1 +
gcc/config/riscv/riscv-v.cc | 18 +++++++++++++++++
gcc/config/riscv/riscv.md | 3 +++
gcc/config/riscv/vector.md | 24 ++++++++++++++---------
5 files changed, 71 insertions(+), 9 deletions(-)
+(define_register_constraint "wo21" "TARGET_VECTOR ? V_REGS : NO_REGS"
+ "A constraint matches the reg group overlap with highest 1 num in 2 regs."
+ "(regno % 2) == 1")
+(define_register_constraint "wo22" "TARGET_VECTOR ? V_REGS : NO_REGS"
+ "A constraint matches the reg group overlap in 2 regs."
+ "(regno % 2) == 0")
So this reminds me a bit of how we deal with auto-inc address modes
where we're not allowed to have the same physical register used in the
auto-inc address and as another input. So think a store to memory
where the source register is also used as part of an auto-inc addressing
mode.
The trick Paul (IIRC) noted is that you can define a set of constraint
pairs say X and Y where X would have just one register and Y would have
all the registers except for one in X. Then you can describe with
constraints the restriction. It's not pretty and not scalable, but it
works for oddballs like pdp11 and h8 where this is a real issue.
You patch uses the same idea, define paired constraints with disjoint
members.
But I'm struggling to see how (regno % 2) == [01] is the right test
here. Let's take your table:
+;; -----------+------------+------------+----------+
+;; | | LMUL = 8 | LMUL = 4 | LMUL = 2 |
+;; +-----------+------------+------------+----------+
+;; | vzext.vf2 | EMUL = 4 | EMUL = 2 | EMUL = 1 |
+;; +-----------+------------+------------+----------+
+;; | | v0-7, v4-7 | v0-3, v2-3 | v0-1, v1 |
If we consider the LMUL=8, EMUL4 case, won't this return true for v0, v2
when only v4-v7 are OK? Or am I missing something?
jeff
