diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h index ac94c6099d080..9a54dbd434d92 100644 --- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h +++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h @@ -479,17 +479,12 @@ class SDNode : public FoldingSetNode, public ilist_node { /// The operation that this node performs. int32_t NodeType; -public: - /// Unique and persistent id per SDNode in the DAG. Used for debug printing. - /// We do not place that under `#if LLVM_ENABLE_ABI_BREAKING_CHECKS` - /// intentionally because it adds unneeded complexity without noticeable - /// benefits (see discussion with @thakis in D120714). - uint16_t PersistentId = 0xffff; + SDNodeFlags Flags; protected: // We define a set of mini-helper classes to help us interpret the bits in our // SubclassData. These are designed to fit within a uint16_t so they pack - // with PersistentId. + // with SDNodeFlags. #if defined(_AIX) && (!defined(__GNUC__) || defined(__clang__)) // Except for GCC; by default, AIX compilers store bit-fields in 4-byte words @@ -611,6 +606,14 @@ END_TWO_BYTE_PACK() static_assert(sizeof(LoadSDNodeBitfields) <= 2, "field too wide"); static_assert(sizeof(StoreSDNodeBitfields) <= 2, "field too wide"); +public: + /// Unique and persistent id per SDNode in the DAG. Used for debug printing. + /// We do not place that under `#if LLVM_ENABLE_ABI_BREAKING_CHECKS` + /// intentionally because it adds unneeded complexity without noticeable + /// benefits (see discussion with @thakis in D120714). Currently, there are + /// two padding bytes after this field. + uint16_t PersistentId = 0xffff; + private: friend class SelectionDAG; // TODO: unfriend HandleSDNode once we fix its operand handling. @@ -646,7 +649,8 @@ END_TWO_BYTE_PACK() /// Return a pointer to the specified value type. static const EVT *getValueTypeList(EVT VT); - SDNodeFlags Flags; + /// Index in worklist of DAGCombiner, or -1. + int CombinerWorklistIndex = -1; uint32_t CFIType = 0; @@ -747,6 +751,12 @@ END_TWO_BYTE_PACK() /// Set unique node id. void setNodeId(int Id) { NodeId = Id; } + /// Get worklist index for DAGCombiner + int getCombinerWorklistIndex() const { return CombinerWorklistIndex; } + + /// Set worklist index for DAGCombiner + void setCombinerWorklistIndex(int Index) { CombinerWorklistIndex = Index; } + /// Return the node ordering. unsigned getIROrder() const { return IROrder; } diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 8607b50175359..90c940e991835 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -170,16 +170,11 @@ namespace { /// back and when processing we pop off of the back. /// /// The worklist will not contain duplicates but may contain null entries - /// due to nodes being deleted from the underlying DAG. + /// due to nodes being deleted from the underlying DAG. For fast lookup and + /// deduplication, the index of the node in this vector is stored in the + /// node in SDNode::CombinerWorklistIndex. SmallVector Worklist; - /// Mapping from an SDNode to its position on the worklist. - /// - /// This is used to find and remove nodes from the worklist (by nulling - /// them) when they are deleted from the underlying DAG. It relies on - /// stable indices of nodes within the worklist. - DenseMap WorklistMap; - /// This records all nodes attempted to be added to the worklist since we /// considered a new worklist entry. As we keep do not add duplicate nodes /// in the worklist, this is different from the tail of the worklist. @@ -238,10 +233,9 @@ namespace { } if (N) { - bool GoodWorklistEntry = WorklistMap.erase(N); - (void)GoodWorklistEntry; - assert(GoodWorklistEntry && + assert(N->getCombinerWorklistIndex() >= 0 && "Found a worklist entry without a corresponding map entry!"); + N->setCombinerWorklistIndex(-1); } return N; } @@ -285,8 +279,10 @@ namespace { if (IsCandidateForPruning) ConsiderForPruning(N); - if (WorklistMap.insert(std::make_pair(N, Worklist.size())).second) + if (N->getCombinerWorklistIndex() == -1) { + N->setCombinerWorklistIndex(Worklist.size()); Worklist.push_back(N); + } } /// Remove all instances of N from the worklist. @@ -295,13 +291,13 @@ namespace { PruningList.remove(N); StoreRootCountMap.erase(N); - auto It = WorklistMap.find(N); - if (It == WorklistMap.end()) + int WorklistIndex = N->getCombinerWorklistIndex(); + if (WorklistIndex == -1) return; // Not in the worklist. // Null out the entry rather than erasing it to avoid a linear operation. - Worklist[It->second] = nullptr; - WorklistMap.erase(It); + Worklist[WorklistIndex] = nullptr; + N->setCombinerWorklistIndex(-1); } void deleteAndRecombine(SDNode *N);