Skip to content

Commit 4b50405

Browse files
author
Kai Lin
committed
[RISCV][DAGCombiner] Fix missed combines in combineOp_VLToVWOp_VL by updating users of new nodes
The previous implementation of combineOp_VLToVWOp_VL updated the DAG by replacing old nodes with newly created widened nodes, but only added the new node itself to the DAGCombiner worklist. Its users were not added, which could cause the external DAGCombiner to miss opportunities to re-run combine logic on dependent nodes. This patch aligns the behavior of the helper's private worklist with the logic used by DAGCombiner::Run when replacing nodes: - Replace all uses of the old node with the new one. - Add both the new node and all of its users to the worklist. - Recursively delete unused nodes. Although this duplicates part of the logic in DAGCombiner::Run, the private worklist in combineOp_VLToVWOp_VL is currently necessary (see PR#168026 for discussion). Splitting this logic into a common utility is desirable but left for future cleanup. This patch reduces behavioral discrepancies between the private worklist and the external DAGCombiner worklist, eliminating missed combine cases.
1 parent f60ec38 commit 4b50405

File tree

2 files changed

+22
-9
lines changed

2 files changed

+22
-9
lines changed

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18300,8 +18300,24 @@ static SDValue combineOp_VLToVWOp_VL(SDNode *N,
1830018300
}
1830118301
}
1830218302
for (std::pair<SDValue, SDValue> OldNewValues : ValuesToReplace) {
18303-
DAG.ReplaceAllUsesOfValueWith(OldNewValues.first, OldNewValues.second);
18304-
DCI.AddToWorklist(OldNewValues.second.getNode());
18303+
// This helper function do as what is done for RV in `DAGCombiner::Run`.
18304+
[&DAG, &DCI](SDNode *N, SDValue RV) -> void {
18305+
if (N->getNumValues() == RV->getNumValues())
18306+
DAG.ReplaceAllUsesWith(N, RV.getNode());
18307+
else {
18308+
assert(N->getValueType(0) == RV.getValueType() &&
18309+
N->getNumValues() == 1 && "Type mismatch");
18310+
DAG.ReplaceAllUsesWith(N, &RV);
18311+
}
18312+
18313+
if (RV.getOpcode() != ISD::EntryToken) {
18314+
DCI.AddToWorklist(RV.getNode());
18315+
for (SDNode *Node : RV.getNode()->users())
18316+
DCI.AddToWorklist(Node);
18317+
}
18318+
18319+
DCI.recursivelyDeleteUnusedNodes(N);
18320+
}(OldNewValues.first.getNode(), OldNewValues.second);
1830518321
}
1830618322
return InputRootReplacement;
1830718323
}

llvm/test/CodeGen/RISCV/rvv/combine-vl-vw-macc.ll

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,16 @@ define void @matmul_min(<32 x i8>* %vptr, i8* %scalars, <32 x i16>* %acc0_ptr, <
77
; CHECK: # %bb.0: # %entry
88
; CHECK-NEXT: li a4, 64
99
; CHECK-NEXT: li a5, 32
10-
; CHECK-NEXT: vsetvli zero, a4, e8, m4, ta, ma
11-
; CHECK-NEXT: vle8.v v8, (a2)
1210
; CHECK-NEXT: vsetvli zero, a5, e8, m2, ta, ma
13-
; CHECK-NEXT: vle8.v v20, (a0)
11+
; CHECK-NEXT: vle8.v v16, (a0)
1412
; CHECK-NEXT: lb a0, 0(a1)
1513
; CHECK-NEXT: lb a1, 1(a1)
1614
; CHECK-NEXT: vsetvli zero, a4, e8, m4, ta, ma
15+
; CHECK-NEXT: vle8.v v8, (a2)
1716
; CHECK-NEXT: vle8.v v12, (a3)
1817
; CHECK-NEXT: vsetvli zero, a5, e8, m2, ta, ma
19-
; CHECK-NEXT: vwmacc.vx v8, a0, v20
20-
; CHECK-NEXT: vwmul.vx v16, v20, a1
21-
; CHECK-NEXT: vsetvli zero, zero, e16, m4, ta, ma
22-
; CHECK-NEXT: vadd.vv v12, v16, v12
18+
; CHECK-NEXT: vwmacc.vx v8, a0, v16
19+
; CHECK-NEXT: vwmacc.vx v12, a1, v16
2320
; CHECK-NEXT: vsetvli zero, a4, e8, m4, ta, ma
2421
; CHECK-NEXT: vse8.v v8, (a2)
2522
; CHECK-NEXT: vse8.v v12, (a3)

0 commit comments

Comments
 (0)