Don't maintain a set of deleted nodes; instead, use a HandleSDNode

to track a node over CSE events. This fixes PR7368.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@106266 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/X86/X86ISelDAGToDAG.cpp

@@ -137,21 +137,6 @@ namespace {
 }
 
 namespace {
-  class X86ISelListener : public SelectionDAG::DAGUpdateListener {
-    SmallSet<SDNode*, 4> Deletes;
-  public:
-    explicit X86ISelListener() {}
-    virtual void NodeDeleted(SDNode *N, SDNode *E) {
-      Deletes.insert(N);
-    }
-    virtual void NodeUpdated(SDNode *N) {
-      // Ignore updates.
-    }
-    bool IsDeleted(SDNode *N) {
-      return Deletes.count(N);
-    }
-  };
-
   //===--------------------------------------------------------------------===//
   /// ISel - X86 specific code to select X86 machine instructions for
   /// SelectionDAG operations.
@@ -199,7 +184,6 @@ namespace {
     bool MatchWrapper(SDValue N, X86ISelAddressMode &AM);
     bool MatchAddress(SDValue N, X86ISelAddressMode &AM);
     bool MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
-                                 X86ISelListener &DeadNodes,
                                  unsigned Depth);
     bool MatchAddressBase(SDValue N, X86ISelAddressMode &AM);
     bool SelectAddr(SDNode *Op, SDValue N, SDValue &Base,
@@ -664,8 +648,7 @@ bool X86DAGToDAGISel::MatchWrapper(SDValue N, X86ISelAddressMode &AM) {
 /// returning true if it cannot be done.  This just pattern matches for the
 /// addressing mode.
 bool X86DAGToDAGISel::MatchAddress(SDValue N, X86ISelAddressMode &AM) {
-  X86ISelListener DeadNodes;
+  if (MatchAddressRecursively(N, AM, 0))
-  if (MatchAddressRecursively(N, AM, DeadNodes, 0))
     return true;
 
   // Post-processing: Convert lea(,%reg,2) to lea(%reg,%reg), which has
@@ -713,7 +696,6 @@ static bool isLogicallyAddWithConstant(SDValue V, SelectionDAG *CurDAG) {
 }
 
 bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
-                                              X86ISelListener &DeadNodes,
                                               unsigned Depth) {
   bool is64Bit = Subtarget->is64Bit();
   DebugLoc dl = N.getDebugLoc();
@@ -876,13 +858,13 @@ bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
     // other uses, since it avoids a two-address sub instruction, however
     // it costs an additional mov if the index register has other uses.
 
+    // Add an artificial use to this node so that we can keep track of
+    // it if it gets CSE'd with a different node.
+    HandleSDNode Handle(N);
+
     // Test if the LHS of the sub can be folded.
     X86ISelAddressMode Backup = AM;
-    if (MatchAddressRecursively(N.getNode()->getOperand(0), AM,
+    if (MatchAddressRecursively(N.getNode()->getOperand(0), AM, Depth+1)) {
-                                DeadNodes, Depth+1) ||
-        // If it is successful but the recursive update causes N to be deleted,
-        // then it's not safe to continue.
-        DeadNodes.IsDeleted(N.getNode())) {
       AM = Backup;
       break;
     }
@@ -893,7 +875,7 @@ bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
     }
 
     int Cost = 0;
-    SDValue RHS = N.getNode()->getOperand(1);
+    SDValue RHS = Handle.getValue().getNode()->getOperand(1);
     // If the RHS involves a register with multiple uses, this
     // transformation incurs an extra mov, due to the neg instruction
     // clobbering its operand.
@@ -944,35 +926,27 @@ bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
   }
 
   case ISD::ADD: {
+    // Add an artificial use to this node so that we can keep track of
+    // it if it gets CSE'd with a different node.
+    HandleSDNode Handle(N);
+    SDValue LHS = Handle.getValue().getNode()->getOperand(0);
+    SDValue RHS = Handle.getValue().getNode()->getOperand(1);
+
     X86ISelAddressMode Backup = AM;
-    if (!MatchAddressRecursively(N.getNode()->getOperand(0), AM,
+    if (!MatchAddressRecursively(LHS, AM, Depth+1) &&
-                                 DeadNodes, Depth+1)) {
+        !MatchAddressRecursively(RHS, AM, Depth+1))
-      if (DeadNodes.IsDeleted(N.getNode()))
+      return false;
-        // If it is successful but the recursive update causes N to be deleted,
+    AM = Backup;
-        // then it's not safe to continue.
+    LHS = Handle.getValue().getNode()->getOperand(0);
-        return true;
+    RHS = Handle.getValue().getNode()->getOperand(1);
-      if (!MatchAddressRecursively(N.getNode()->getOperand(1), AM,
-                                   DeadNodes, Depth+1))
-        // If it is successful but the recursive update causes N to be deleted,
-        // then it's not safe to continue.
-        return DeadNodes.IsDeleted(N.getNode());
-    }
 
     // Try again after commuting the operands.
+    if (!MatchAddressRecursively(RHS, AM, Depth+1) &&
+        !MatchAddressRecursively(LHS, AM, Depth+1))
+      return false;
     AM = Backup;
-    if (!MatchAddressRecursively(N.getNode()->getOperand(1), AM,
+    LHS = Handle.getValue().getNode()->getOperand(0);
-                                 DeadNodes, Depth+1)) {
+    RHS = Handle.getValue().getNode()->getOperand(1);
-      if (DeadNodes.IsDeleted(N.getNode()))
-        // If it is successful but the recursive update causes N to be deleted,
-        // then it's not safe to continue.
-        return true;
-      if (!MatchAddressRecursively(N.getNode()->getOperand(0), AM,
-                                   DeadNodes, Depth+1))
-        // If it is successful but the recursive update causes N to be deleted,
-        // then it's not safe to continue.
-        return DeadNodes.IsDeleted(N.getNode());
-    }
-    AM = Backup;
 
     // If we couldn't fold both operands into the address at the same time,
     // see if we can just put each operand into a register and fold at least
@@ -980,8 +954,8 @@ bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
     if (AM.BaseType == X86ISelAddressMode::RegBase &&
         !AM.Base_Reg.getNode() &&
         !AM.IndexReg.getNode()) {
-      AM.Base_Reg = N.getNode()->getOperand(0);
+      AM.Base_Reg = LHS;
-      AM.IndexReg = N.getNode()->getOperand(1);
+      AM.IndexReg = RHS;
       AM.Scale = 1;
       return false;
     }
@@ -996,7 +970,7 @@ bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
       uint64_t Offset = CN->getSExtValue();
 
       // Start with the LHS as an addr mode.
-      if (!MatchAddressRecursively(N.getOperand(0), AM, DeadNodes, Depth+1) &&
+      if (!MatchAddressRecursively(N.getOperand(0), AM, Depth+1) &&
           // Address could not have picked a GV address for the displacement.
           AM.GV == NULL &&
           // On x86-64, the resultant disp must fit in 32-bits.
@@ -1073,7 +1047,7 @@ bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
           CurDAG->RepositionNode(N.getNode(), Shl.getNode());
           Shl.getNode()->setNodeId(N.getNode()->getNodeId());
         }
-        CurDAG->ReplaceAllUsesWith(N, Shl, &DeadNodes);
+        CurDAG->ReplaceAllUsesWith(N, Shl);
         AM.IndexReg = And;
         AM.Scale = (1 << ScaleLog);
         return false;
@@ -1124,7 +1098,7 @@ bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
       NewSHIFT.getNode()->setNodeId(N.getNode()->getNodeId());
     }
 
-    CurDAG->ReplaceAllUsesWith(N, NewSHIFT, &DeadNodes);
+    CurDAG->ReplaceAllUsesWith(N, NewSHIFT);
     
     AM.Scale = 1 << ShiftCst;
     AM.IndexReg = NewAND;

test/CodeGen/X86/2010-03-17-ISelBug.ll

@@ -1,4 +1,5 @@
 ; RUN: llc < %s -mtriple=i386-apple-darwin5
+
 ; rdar://7761790
 
 %"struct..0$_485" = type { i16, i16, i32 }
@@ -37,3 +38,30 @@ bb169:                                            ; preds = %bb169, %bb.nph380
   %4 = add nsw i32 %index.6379, 1                 ; <i32> [#uses=1]
   br label %bb169
 }
+
+; PR7368
+
+%struct.bufBit_s = type { i8*, i8 }
+
+define fastcc void @printSwipe([2 x [256 x %struct.bufBit_s]]* nocapture %colourLines) nounwind {
+entry:
+  br label %for.body190
+  
+for.body261.i:                                    ; preds = %for.body261.i, %for.body190
+  %line.3300.i = phi i32 [ undef, %for.body190 ], [ %add292.i, %for.body261.i ] ; <i32> [#uses=3]
+  %conv268.i = and i32 %line.3300.i, 255          ; <i32> [#uses=1]
+  %tmp278.i = getelementptr [2 x [256 x %struct.bufBit_s]]* %colourLines, i32 undef, i32 %pen.1100, i32 %conv268.i, i32 0 ; <i8**> [#uses=1]
+  store i8* undef, i8** %tmp278.i
+  %tmp338 = shl i32 %line.3300.i, 3               ; <i32> [#uses=1]
+  %tmp339 = and i32 %tmp338, 2040                 ; <i32> [#uses=1]
+  %tmp285.i = getelementptr i8* %scevgep328, i32 %tmp339 ; <i8*> [#uses=1]
+  store i8 undef, i8* %tmp285.i
+  %add292.i = add nsw i32 0, %line.3300.i         ; <i32> [#uses=1]
+  br i1 undef, label %for.body190, label %for.body261.i
+
+for.body190:                                      ; preds = %for.body261.i, %for.body190, %bb.nph104
+  %pen.1100 = phi i32 [ 0, %entry ], [ %inc230, %for.body261.i ], [ %inc230, %for.body190 ] ; <i32> [#uses=3]
+  %scevgep328 = getelementptr [2 x [256 x %struct.bufBit_s]]* %colourLines, i32 undef, i32 %pen.1100, i32 0, i32 1 ; <i8*> [#uses=1]
+  %inc230 = add i32 %pen.1100, 1                  ; <i32> [#uses=2]
+  br i1 undef, label %for.body190, label %for.body261.i
+}