Skip to content

[RISCV] Add disjoint or patterns for vwadd[u].vv #136716

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 23, 2025
Merged

[RISCV] Add disjoint or patterns for vwadd[u].vv #136716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6cd1750
[RISCV] Add disjoint or patterns for vwadd[u].vv
lukel97 Apr 22, 2025
644fe31
Don't propagate disjoint for inreg exts, use or_is_add, match SplatPat
lukel97 Apr 22, 2025
f69c8e6
Update multiclass name to include VX
lukel97 Apr 23, 2025
9599e60
Remove extraneous newline
lukel97 Apr 23, 2025
288fb1f
Fix records
lukel97 Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5982,7 +5982,10 @@ SDValue DAGCombiner::hoistLogicOpWithSameOpcodeHands(SDNode *N) {
LegalTypes && !TLI.isTypeDesirableForOp(LogicOpcode, XVT))
return SDValue();
// logic_op (hand_op X), (hand_op Y) --> hand_op (logic_op X, Y)
SDValue Logic = DAG.getNode(LogicOpcode, DL, XVT, X, Y);
SDNodeFlags LogicFlags;
LogicFlags.setDisjoint(N->getFlags().hasDisjoint() &&
Copy link
Collaborator

@preames preames Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure I understand this right. We've got (or disjoint (ext a), (ext b)) and we're turning that into (ext (or disjoint a, b)). Right?

For zext, this is fine. For sext, this is fine. For the in_reg variants, I am not sure if this is fine or not. What if a and b had the same high bit set and we're doing a sext_in_reg which clears that bit? I think that violates the disjoint and introduces UB which didn't previously exist right?

Copy link
Contributor Author

@lukel97 lukel97 Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agh good catch, alive2 does indeed confirm this: https://alive2.llvm.org/ce/z/W9F5b8. Will fix

ISD::isExtOpcode(HandOpcode));
SDValue Logic = DAG.getNode(LogicOpcode, DL, XVT, X, Y, LogicFlags);
if (HandOpcode == ISD::SIGN_EXTEND_INREG)
return DAG.getNode(HandOpcode, DL, VT, Logic, N0.getOperand(1));
return DAG.getNode(HandOpcode, DL, VT, Logic);
Expand Down
23 changes: 23 additions & 0 deletions llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,29 @@ defm : VPatWidenBinarySDNode_VV_VX_WV_WX<add, sext_oneuse, "PseudoVWADD">;
defm : VPatWidenBinarySDNode_VV_VX_WV_WX<add, zext_oneuse, "PseudoVWADDU">;
defm : VPatWidenBinarySDNode_VV_VX_WV_WX<add, anyext_oneuse, "PseudoVWADDU">;

// DAGCombiner::hoistLogicOpWithSameOpcodeHands may hoist disjoint ors
Copy link
Collaborator

@preames preames Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have saying this - but remember this only covers scalable types, and that you probably need a follow up patch for the fixed length variant.

// to (ext (or disjoint (a, b)))
multiclass VPatWidenOrDisjoint_VV_VX<SDNode extop, string instruction_name> {
foreach vtiToWti = AllWidenableIntVectors in {
defvar vti = vtiToWti.Vti;
defvar wti = vtiToWti.Wti;
let Predicates = !listconcat(GetVTypePredicates<vti>.Predicates,
GetVTypePredicates<wti>.Predicates) in {
def : Pat<(wti.Vector (extop (vti.Vector (or_is_add vti.RegClass:$rs2, vti.RegClass:$rs1)))),
(!cast<Instruction>(instruction_name#"_VV_"#vti.LMul.MX)
(wti.Vector (IMPLICIT_DEF)), vti.RegClass:$rs2,
vti.RegClass:$rs1, vti.AVL, vti.Log2SEW, TA_MA)>;
def : Pat<(wti.Vector (extop (vti.Vector (or_is_add vti.RegClass:$rs2, (SplatPat (XLenVT GPR:$rs1)))))),
(!cast<Instruction>(instruction_name#"_VX_"#vti.LMul.MX)
(wti.Vector (IMPLICIT_DEF)), vti.RegClass:$rs2,
GPR:$rs1, vti.AVL, vti.Log2SEW, TA_MA)>;
}
}
}
defm : VPatWidenOrDisjoint_VV_VX<sext, "PseudoVWADD">;
defm : VPatWidenOrDisjoint_VV_VX<zext, "PseudoVWADDU">;
defm : VPatWidenOrDisjoint_VV_VX<anyext, "PseudoVWADDU">;

defm : VPatWidenBinarySDNode_VV_VX_WV_WX<sub, sext_oneuse, "PseudoVWSUB">;
defm : VPatWidenBinarySDNode_VV_VX_WV_WX<sub, zext_oneuse, "PseudoVWSUBU">;
defm : VPatWidenBinarySDNode_VV_VX_WV_WX<sub, anyext_oneuse, "PseudoVWSUBU">;
Expand Down
44 changes: 34 additions & 10 deletions llvm/test/CodeGen/RISCV/rvv/vwadd-sdnode.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1417,38 +1417,62 @@ define <vscale x 2 x i32> @vwaddu_vv_disjoint_or_add(<vscale x 2 x i8> %x.i8, <v
ret <vscale x 2 x i32> %add
}

; TODO: We could select vwaddu.vv, but when both arms of the or are the same
; DAGCombiner::hoistLogicOpWithSameOpcodeHands moves the zext above the or.
define <vscale x 2 x i32> @vwaddu_vv_disjoint_or(<vscale x 2 x i16> %x.i16, <vscale x 2 x i16> %y.i16) {
; CHECK-LABEL: vwaddu_vv_disjoint_or:
; CHECK: # %bb.0:
; CHECK-NEXT: vsetvli a0, zero, e16, mf2, ta, ma
; CHECK-NEXT: vor.vv v9, v8, v9
; CHECK-NEXT: vsetvli zero, zero, e32, m1, ta, ma
; CHECK-NEXT: vzext.vf2 v8, v9
; CHECK-NEXT: vwaddu.vv v10, v8, v9
; CHECK-NEXT: vmv1r.v v8, v10
; CHECK-NEXT: ret
%x.i32 = zext <vscale x 2 x i16> %x.i16 to <vscale x 2 x i32>
%y.i32 = zext <vscale x 2 x i16> %y.i16 to <vscale x 2 x i32>
%or = or disjoint <vscale x 2 x i32> %x.i32, %y.i32
ret <vscale x 2 x i32> %or
}

; TODO: We could select vwadd.vv, but when both arms of the or are the same
; DAGCombiner::hoistLogicOpWithSameOpcodeHands moves the zext above the or.
define <vscale x 2 x i32> @vwadd_vv_disjoint_or(<vscale x 2 x i16> %x.i16, <vscale x 2 x i16> %y.i16) {
; CHECK-LABEL: vwadd_vv_disjoint_or:
; CHECK: # %bb.0:
; CHECK-NEXT: vsetvli a0, zero, e16, mf2, ta, ma
; CHECK-NEXT: vor.vv v9, v8, v9
; CHECK-NEXT: vsetvli zero, zero, e32, m1, ta, ma
; CHECK-NEXT: vsext.vf2 v8, v9
; CHECK-NEXT: vwadd.vv v10, v8, v9
; CHECK-NEXT: vmv1r.v v8, v10
; CHECK-NEXT: ret
%x.i32 = sext <vscale x 2 x i16> %x.i16 to <vscale x 2 x i32>
%y.i32 = sext <vscale x 2 x i16> %y.i16 to <vscale x 2 x i32>
%or = or disjoint <vscale x 2 x i32> %x.i32, %y.i32
ret <vscale x 2 x i32> %or
}

define <vscale x 2 x i32> @vwaddu_vx_disjoint_or(<vscale x 2 x i16> %x.i16, i16 %y.i16) {
; CHECK-LABEL: vwaddu_vx_disjoint_or:
; CHECK: # %bb.0:
; CHECK-NEXT: vsetvli a1, zero, e16, mf2, ta, ma
; CHECK-NEXT: vwaddu.vx v9, v8, a0
; CHECK-NEXT: vmv1r.v v8, v9
; CHECK-NEXT: ret
%x.i32 = zext <vscale x 2 x i16> %x.i16 to <vscale x 2 x i32>
%y.head = insertelement <vscale x 2 x i16> poison, i16 %y.i16, i32 0
%y.splat = shufflevector <vscale x 2 x i16> %y.head, <vscale x 2 x i16> poison, <vscale x 2 x i32> zeroinitializer
%y.i32 = zext <vscale x 2 x i16> %y.splat to <vscale x 2 x i32>
%or = or disjoint <vscale x 2 x i32> %x.i32, %y.i32
ret <vscale x 2 x i32> %or
}

define <vscale x 2 x i32> @vwadd_vx_disjoint_or(<vscale x 2 x i16> %x.i16, i16 %y.i16) {
; CHECK-LABEL: vwadd_vx_disjoint_or:
; CHECK: # %bb.0:
; CHECK-NEXT: vsetvli a1, zero, e16, mf2, ta, ma
; CHECK-NEXT: vwadd.vx v9, v8, a0
; CHECK-NEXT: vmv1r.v v8, v9
; CHECK-NEXT: ret
%x.i32 = sext <vscale x 2 x i16> %x.i16 to <vscale x 2 x i32>
%y.head = insertelement <vscale x 2 x i16> poison, i16 %y.i16, i32 0
%y.splat = shufflevector <vscale x 2 x i16> %y.head, <vscale x 2 x i16> poison, <vscale x 2 x i32> zeroinitializer
%y.i32 = sext <vscale x 2 x i16> %y.splat to <vscale x 2 x i32>
%or = or disjoint <vscale x 2 x i32> %x.i32, %y.i32
ret <vscale x 2 x i32> %or
}

define <vscale x 2 x i32> @vwaddu_wv_disjoint_or(<vscale x 2 x i32> %x.i32, <vscale x 2 x i16> %y.i16) {
; CHECK-LABEL: vwaddu_wv_disjoint_or:
; CHECK: # %bb.0:
Expand Down
Loading