[llvm] r214546 - [SDAG] Begin simplifying the way in which the legalizer deletes nodes.

Chandler Carruth chandlerc at gmail.com
Fri Aug 1 12:50:00 PDT 2014


Author: chandlerc
Date: Fri Aug  1 14:49:59 2014
New Revision: 214546

URL: http://llvm.org/viewvc/llvm-project?rev=214546&view=rev
Log:
[SDAG] Begin simplifying the way in which the legalizer deletes nodes.

This lifts the (very few) places the legalizer would delete dead nodes
into the outer loop around the legalizer. This is significantly simpler
because it doesn't require the legalizer itself to manage the iterator
validity, and it doesn't require the legalizer to be a DAG update
listener in order to remove things from the legalized set. It also makes
the interface much less contrived for the case of the legalizer running
inside the last phase of DAG combining.

I'm working on centralizing the deletion of nodes during both legalizing
and combining as much as possible. My hope is to remove the need for DAG
update listeners from the combiner next, which would remove a costly
virtual dispatch chain on every deletion. This in turn should allow us
to more aggressively delete DAG nodes during combining which will in
turn allow us to combine more aggressively by exposing the actual nodes
which have single users to the combine phases.

Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp?rev=214546&r1=214545&r2=214546&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp Fri Aug  1 14:49:59 2014
@@ -51,15 +51,11 @@ using namespace llvm;
 /// will attempt merge setcc and brc instructions into brcc's.
 ///
 namespace {
-class SelectionDAGLegalize : public SelectionDAG::DAGUpdateListener {
+class SelectionDAGLegalize {
   const TargetMachine &TM;
   const TargetLowering &TLI;
   SelectionDAG &DAG;
 
-  /// \brief The iterator being used to walk the DAG. We hold a reference to it
-  /// in order to update it as necessary on node deletion.
-  SelectionDAG::allnodes_iterator &LegalizePosition;
-
   /// \brief The set of nodes which have already been legalized. We hold a
   /// reference to it in order to update as necessary on node deletion.
   SmallPtrSetImpl<SDNode *> &LegalizedNodes;
@@ -75,13 +71,10 @@ class SelectionDAGLegalize : public Sele
 
 public:
   SelectionDAGLegalize(SelectionDAG &DAG,
-                       SelectionDAG::allnodes_iterator &LegalizePosition,
                        SmallPtrSetImpl<SDNode *> &LegalizedNodes,
                        SmallSetVector<SDNode *, 16> *UpdatedNodes = nullptr)
-      : SelectionDAG::DAGUpdateListener(DAG), TM(DAG.getTarget()),
-        TLI(DAG.getTargetLoweringInfo()), DAG(DAG),
-        LegalizePosition(LegalizePosition), LegalizedNodes(LegalizedNodes),
-        UpdatedNodes(UpdatedNodes) {}
+      : TM(DAG.getTarget()), TLI(DAG.getTargetLoweringInfo()), DAG(DAG),
+        LegalizedNodes(LegalizedNodes), UpdatedNodes(UpdatedNodes) {}
 
   /// \brief Legalizes the given operation.
   void LegalizeOp(SDNode *Node);
@@ -158,28 +151,10 @@ private:
   void ExpandNode(SDNode *Node);
   void PromoteNode(SDNode *Node);
 
-  void ForgetNode(SDNode *N) {
-    LegalizedNodes.erase(N);
-    if (LegalizePosition == SelectionDAG::allnodes_iterator(N))
-      ++LegalizePosition;
-    if (UpdatedNodes)
-      UpdatedNodes->remove(N);
-  }
-
 public:
-  // DAGUpdateListener implementation.
-  void NodeDeleted(SDNode *N, SDNode *E) override {
-    ForgetNode(N);
-  }
-  void NodeUpdated(SDNode *N) override {}
-
   // Node replacement helpers
   void ReplacedNode(SDNode *N) {
-    if (N->use_empty()) {
-      DAG.RemoveDeadNode(N);
-    } else {
-      ForgetNode(N);
-    }
+    LegalizedNodes.erase(N);
   }
   void ReplaceNode(SDNode *Old, SDNode *New) {
     DEBUG(dbgs() << " ... replacing: "; Old->dump(&DAG);
@@ -3830,9 +3805,11 @@ void SelectionDAGLegalize::ExpandNode(SD
       TopHalf = DAG.getNode(ISD::EXTRACT_ELEMENT, dl, VT, Ret,
                             DAG.getIntPtrConstant(1));
       // Ret is a node with an illegal type. Because such things are not
-      // generally permitted during this phase of legalization, delete the
-      // node. The above EXTRACT_ELEMENT nodes should have been folded.
-      DAG.DeleteNode(Ret.getNode());
+      // generally permitted during this phase of legalization, make sure the
+      // node has no more uses. The above EXTRACT_ELEMENT nodes should have been
+      // folded.
+      assert(Ret->use_empty() &&
+             "Unexpected uses of illegally type from expanded lib call.");
     }
 
     if (isSigned) {
@@ -4315,9 +4292,8 @@ void SelectionDAGLegalize::PromoteNode(S
 void SelectionDAG::Legalize() {
   AssignTopologicalOrder();
 
-  allnodes_iterator LegalizePosition;
   SmallPtrSet<SDNode *, 16> LegalizedNodes;
-  SelectionDAGLegalize Legalizer(*this, LegalizePosition, LegalizedNodes);
+  SelectionDAGLegalize Legalizer(*this, LegalizedNodes);
 
   // Visit all the nodes. We start in topological order, so that we see
   // nodes with their original operands intact. Legalization can produce
@@ -4325,14 +4301,24 @@ void SelectionDAG::Legalize() {
   // nodes have been legalized.
   for (;;) {
     bool AnyLegalized = false;
-    for (LegalizePosition = allnodes_end();
-         LegalizePosition != allnodes_begin(); ) {
-      --LegalizePosition;
+    for (auto NI = allnodes_end(); NI != allnodes_begin();) {
+      --NI;
+
+      SDNode *N = NI;
+      if (N->use_empty() && N != getRoot().getNode()) {
+        ++NI;
+        DeleteNode(N);
+        continue;
+      }
 
-      SDNode *N = LegalizePosition;
       if (LegalizedNodes.insert(N)) {
         AnyLegalized = true;
         Legalizer.LegalizeOp(N);
+
+        if (N->use_empty() && N != getRoot().getNode()) {
+          ++NI;
+          DeleteNode(N);
+        }
       }
     }
     if (!AnyLegalized)
@@ -4346,10 +4332,8 @@ void SelectionDAG::Legalize() {
 
 bool SelectionDAG::LegalizeOp(SDNode *N,
                               SmallSetVector<SDNode *, 16> &UpdatedNodes) {
-  allnodes_iterator LegalizePosition(N);
   SmallPtrSet<SDNode *, 16> LegalizedNodes;
-  SelectionDAGLegalize Legalizer(*this, LegalizePosition, LegalizedNodes,
-                                 &UpdatedNodes);
+  SelectionDAGLegalize Legalizer(*this, LegalizedNodes, &UpdatedNodes);
 
   // Directly insert the node in question, and legalize it. This will recurse
   // as needed through operands.





More information about the llvm-commits mailing list