Skip to content

Commit 74d62c2

Browse files
authored
[CodeGen][SDAG] Remove CombinedNodes SmallPtrSet (#94609)
This "small" set grows quite large and it's more performant to store whether a node has been combined before in the node itself. As this information is only relevant for nodes that are currently not in the worklist, add a second state to the CombinerWorklistIndex (-2) to indicate that a node is currently not in a worklist, but was combined before. This brings a substantial performance improvement.
1 parent 3fefb3c commit 74d62c2

File tree

2 files changed

+20
-17
lines changed

2 files changed

+20
-17
lines changed

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,9 @@ END_TWO_BYTE_PACK()
649649
/// Return a pointer to the specified value type.
650650
static const EVT *getValueTypeList(EVT VT);
651651

652-
/// Index in worklist of DAGCombiner, or -1.
652+
/// Index in worklist of DAGCombiner, or negative if the node is not in the
653+
/// worklist. -1 = not in worklist; -2 = not in worklist, but has already been
654+
/// combined at least once.
653655
int CombinerWorklistIndex = -1;
654656

655657
uint32_t CFIType = 0;

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

+17-16
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,6 @@ namespace {
180180
/// in the worklist, this is different from the tail of the worklist.
181181
SmallSetVector<SDNode *, 32> PruningList;
182182

183-
/// Set of nodes which have been combined (at least once).
184-
///
185-
/// This is used to allow us to reliably add any operands of a DAG node
186-
/// which have not yet been combined to the worklist.
187-
SmallPtrSet<SDNode *, 32> CombinedNodes;
188-
189183
/// Map from candidate StoreNode to the pair of RootNode and count.
190184
/// The count is used to track how many times we have seen the StoreNode
191185
/// with the same RootNode bail out in dependence check. If we have seen
@@ -235,7 +229,8 @@ namespace {
235229
if (N) {
236230
assert(N->getCombinerWorklistIndex() >= 0 &&
237231
"Found a worklist entry without a corresponding map entry!");
238-
N->setCombinerWorklistIndex(-1);
232+
// Set to -2 to indicate that we combined the node.
233+
N->setCombinerWorklistIndex(-2);
239234
}
240235
return N;
241236
}
@@ -267,7 +262,8 @@ namespace {
267262

268263
/// Add to the worklist making sure its instance is at the back (next to be
269264
/// processed.)
270-
void AddToWorklist(SDNode *N, bool IsCandidateForPruning = true) {
265+
void AddToWorklist(SDNode *N, bool IsCandidateForPruning = true,
266+
bool SkipIfCombinedBefore = false) {
271267
assert(N->getOpcode() != ISD::DELETED_NODE &&
272268
"Deleted Node added to Worklist");
273269

@@ -276,23 +272,28 @@ namespace {
276272
if (N->getOpcode() == ISD::HANDLENODE)
277273
return;
278274

275+
if (SkipIfCombinedBefore && N->getCombinerWorklistIndex() == -2)
276+
return;
277+
279278
if (IsCandidateForPruning)
280279
ConsiderForPruning(N);
281280

282-
if (N->getCombinerWorklistIndex() == -1) {
281+
if (N->getCombinerWorklistIndex() < 0) {
283282
N->setCombinerWorklistIndex(Worklist.size());
284283
Worklist.push_back(N);
285284
}
286285
}
287286

288287
/// Remove all instances of N from the worklist.
289288
void removeFromWorklist(SDNode *N) {
290-
CombinedNodes.erase(N);
291289
PruningList.remove(N);
292290
StoreRootCountMap.erase(N);
293291

294292
int WorklistIndex = N->getCombinerWorklistIndex();
295-
if (WorklistIndex == -1)
293+
// If not in the worklist, the index might be -1 or -2 (was combined
294+
// before). As the node gets deleted anyway, there's no need to update
295+
// the index.
296+
if (WorklistIndex < 0)
296297
return; // Not in the worklist.
297298

298299
// Null out the entry rather than erasing it to avoid a linear operation.
@@ -1768,13 +1769,13 @@ void DAGCombiner::Run(CombineLevel AtLevel) {
17681769
LLVM_DEBUG(dbgs() << "\nCombining: "; N->dump(&DAG));
17691770

17701771
// Add any operands of the new node which have not yet been combined to the
1771-
// worklist as well. Because the worklist uniques things already, this
1772-
// won't repeatedly process the same operand.
1772+
// worklist as well. getNextWorklistEntry flags nodes that have been
1773+
// combined before. Because the worklist uniques things already, this won't
1774+
// repeatedly process the same operand.
17731775
for (const SDValue &ChildN : N->op_values())
1774-
if (!CombinedNodes.count(ChildN.getNode()))
1775-
AddToWorklist(ChildN.getNode());
1776+
AddToWorklist(ChildN.getNode(), /*IsCandidateForPruning=*/true,
1777+
/*SkipIfCombinedBefore=*/true);
17761778

1777-
CombinedNodes.insert(N);
17781779
SDValue RV = combine(N);
17791780

17801781
if (!RV.getNode())

0 commit comments

Comments
 (0)