Skip to content

[CodeGen][SDAG] Remove CombinedNodes SmallPtrSet #94609

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.

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 1 commit into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion llvm/include/llvm/CodeGen/SelectionDAGNodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,9 @@ END_TWO_BYTE_PACK()
/// Return a pointer to the specified value type.
static const EVT *getValueTypeList(EVT VT);

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

uint32_t CFIType = 0;
Expand Down
33 changes: 17 additions & 16 deletions llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,6 @@ namespace {
/// in the worklist, this is different from the tail of the worklist.
SmallSetVector<SDNode *, 32> PruningList;

/// Set of nodes which have been combined (at least once).
///
/// This is used to allow us to reliably add any operands of a DAG node
/// which have not yet been combined to the worklist.
SmallPtrSet<SDNode *, 32> CombinedNodes;

/// Map from candidate StoreNode to the pair of RootNode and count.
/// The count is used to track how many times we have seen the StoreNode
/// with the same RootNode bail out in dependence check. If we have seen
Expand Down Expand Up @@ -235,7 +229,8 @@ namespace {
if (N) {
assert(N->getCombinerWorklistIndex() >= 0 &&
"Found a worklist entry without a corresponding map entry!");
N->setCombinerWorklistIndex(-1);
// Set to -2 to indicate that we combined the node.
N->setCombinerWorklistIndex(-2);
}
return N;
}
Expand Down Expand Up @@ -267,7 +262,8 @@ namespace {

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

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

if (SkipIfCombinedBefore && N->getCombinerWorklistIndex() == -2)
return;

if (IsCandidateForPruning)
ConsiderForPruning(N);

if (N->getCombinerWorklistIndex() == -1) {
if (N->getCombinerWorklistIndex() < 0) {
N->setCombinerWorklistIndex(Worklist.size());
Worklist.push_back(N);
}
}

/// Remove all instances of N from the worklist.
void removeFromWorklist(SDNode *N) {
CombinedNodes.erase(N);
PruningList.remove(N);
StoreRootCountMap.erase(N);

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

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

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

CombinedNodes.insert(N);
SDValue RV = combine(N);

if (!RV.getNode())
Expand Down
Loading