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
