[llvm-commits] [llvm] r98780 - in /llvm/trunk: lib/Target/X86/X86ISelDAGToDAG.cpp test/CodeGen/X86/2010-03-17-ISelBug.ll

Evan Cheng evan.cheng at apple.com
Wed Mar 17 16:58:36 PDT 2010


Author: evancheng
Date: Wed Mar 17 18:58:35 2010
New Revision: 98780

URL: http://llvm.org/viewvc/llvm-project?rev=98780&view=rev
Log:
X86 address mode matching code MatchAddressRecursively does some aggressive hack which require doing a RAUW. It may end up deleting some SDNode up stream. It should avoid referencing deleted nodes.

Added:
    llvm/trunk/test/CodeGen/X86/2010-03-17-ISelBug.ll
Modified:
    llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp

Modified: llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp?rev=98780&r1=98779&r2=98780&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp Wed Mar 17 18:58:35 2010
@@ -140,6 +140,21 @@
 }
 
 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.
@@ -187,6 +202,7 @@
     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,
@@ -651,7 +667,8 @@
 /// returning true if it cannot be done.  This just pattern matches for the
 /// addressing mode.
 bool X86DAGToDAGISel::MatchAddress(SDValue N, X86ISelAddressMode &AM) {
-  if (MatchAddressRecursively(N, AM, 0))
+  X86ISelListener DeadNodes;
+  if (MatchAddressRecursively(N, AM, DeadNodes, 0))
     return true;
 
   // Post-processing: Convert lea(,%reg,2) to lea(%reg,%reg), which has
@@ -680,6 +697,7 @@
 }
 
 bool X86DAGToDAGISel::MatchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
+                                              X86ISelListener &DeadNodes,
                                               unsigned Depth) {
   bool is64Bit = Subtarget->is64Bit();
   DebugLoc dl = N.getDebugLoc();
@@ -845,7 +863,11 @@
 
     // Test if the LHS of the sub can be folded.
     X86ISelAddressMode Backup = AM;
-    if (MatchAddressRecursively(N.getNode()->getOperand(0), AM, Depth+1)) {
+    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.
+        DeadNodes.IsDeleted(N.getNode())) {
       AM = Backup;
       break;
     }
@@ -854,6 +876,7 @@
       AM = Backup;
       break;
     }
+
     int Cost = 0;
     SDValue RHS = N.getNode()->getOperand(1);
     // If the RHS involves a register with multiple uses, this
@@ -907,13 +930,33 @@
 
   case ISD::ADD: {
     X86ISelAddressMode Backup = AM;
-    if (!MatchAddressRecursively(N.getNode()->getOperand(0), AM, Depth+1) &&
-        !MatchAddressRecursively(N.getNode()->getOperand(1), AM, Depth+1))
-      return false;
+    if (!MatchAddressRecursively(N.getNode()->getOperand(0), AM,
+                                 DeadNodes, Depth+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(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.
     AM = Backup;
-    if (!MatchAddressRecursively(N.getNode()->getOperand(1), AM, Depth+1) &&
-        !MatchAddressRecursively(N.getNode()->getOperand(0), AM, Depth+1))
-      return false;
+    if (!MatchAddressRecursively(N.getNode()->getOperand(1), AM,
+                                 DeadNodes, Depth+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,
@@ -935,16 +978,19 @@
     if (ConstantSDNode *CN = dyn_cast<ConstantSDNode>(N.getOperand(1))) {
       X86ISelAddressMode Backup = AM;
       uint64_t Offset = CN->getSExtValue();
+
+      // Check to see if the LHS & C is zero.
+      if (!CurDAG->MaskedValueIsZero(N.getOperand(0), CN->getAPIntValue()))
+        break;
+
       // Start with the LHS as an addr mode.
-      if (!MatchAddressRecursively(N.getOperand(0), AM, Depth+1) &&
+      if (!MatchAddressRecursively(N.getOperand(0), AM, DeadNodes, 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.
           (!is64Bit ||
            X86::isOffsetSuitableForCodeModel(AM.Disp + Offset, M,
-                                             AM.hasSymbolicDisplacement())) &&
-          // Check to see if the LHS & C is zero.
-          CurDAG->MaskedValueIsZero(N.getOperand(0), CN->getAPIntValue())) {
+                                             AM.hasSymbolicDisplacement()))) {
         AM.Disp += Offset;
         return false;
       }
@@ -1015,7 +1061,7 @@
           CurDAG->RepositionNode(N.getNode(), Shl.getNode());
           Shl.getNode()->setNodeId(N.getNode()->getNodeId());
         }
-        CurDAG->ReplaceAllUsesWith(N, Shl);
+        CurDAG->ReplaceAllUsesWith(N, Shl, &DeadNodes);
         AM.IndexReg = And;
         AM.Scale = (1 << ScaleLog);
         return false;
@@ -1066,7 +1112,7 @@
       NewSHIFT.getNode()->setNodeId(N.getNode()->getNodeId());
     }
 
-    CurDAG->ReplaceAllUsesWith(N, NewSHIFT);
+    CurDAG->ReplaceAllUsesWith(N, NewSHIFT, &DeadNodes);
     
     AM.Scale = 1 << ShiftCst;
     AM.IndexReg = NewAND;

Added: llvm/trunk/test/CodeGen/X86/2010-03-17-ISelBug.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2010-03-17-ISelBug.ll?rev=98780&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/2010-03-17-ISelBug.ll (added)
+++ llvm/trunk/test/CodeGen/X86/2010-03-17-ISelBug.ll Wed Mar 17 18:58:35 2010
@@ -0,0 +1,39 @@
+; RUN: llc < %s -mtriple=i386-apple-darwin5
+; rdar://7761790
+
+%"struct..0$_485" = type { i16, i16, i32 }
+%union.PPToken = type { %"struct..0$_485" }
+%struct.PPOperation = type { %union.PPToken, %union.PPToken, [6 x %union.PPToken], i32, i32, i32, [1 x i32], [0 x i8] }
+
+define i32* @t() align 2 nounwind {
+entry:
+  %operation = alloca %struct.PPOperation, align 8 ; <%struct.PPOperation*> [#uses=2]
+  %0 = load i32*** null, align 4  ; [#uses=1]
+  %1 = ptrtoint i32** %0 to i32   ; <i32> [#uses=1]
+  %2 = sub nsw i32 %1, undef                      ; <i32> [#uses=2]
+  br i1 false, label %bb20, label %bb.nph380
+
+bb20:                                             ; preds = %entry
+  ret i32* null
+
+bb.nph380:                                        ; preds = %entry
+  %scevgep403 = getelementptr %struct.PPOperation* %operation, i32 0, i32 1, i32 0, i32 2 ; <i32*> [#uses=1]
+  %3 = ashr i32 %2, 1                             ; <i32> [#uses=1]
+  %tmp405 = and i32 %3, -2                        ; <i32> [#uses=1]
+  %scevgep408 = getelementptr %struct.PPOperation* %operation, i32 0, i32 1, i32 0, i32 1 ; <i16*> [#uses=1]
+  %tmp410 = and i32 %2, -4                        ; <i32> [#uses=1]
+  br label %bb169
+
+bb169:                                            ; preds = %bb169, %bb.nph380
+  %index.6379 = phi i32 [ 0, %bb.nph380 ], [ %4, %bb169 ] ; <i32> [#uses=3]
+  %tmp404 = mul i32 %index.6379, -2               ; <i32> [#uses=1]
+  %tmp406 = add i32 %tmp405, %tmp404              ; <i32> [#uses=1]
+  %scevgep407 = getelementptr i32* %scevgep403, i32 %tmp406 ; <i32*> [#uses=1]
+  %tmp409 = mul i32 %index.6379, -4               ; <i32> [#uses=1]
+  %tmp411 = add i32 %tmp410, %tmp409              ; <i32> [#uses=1]
+  %scevgep412 = getelementptr i16* %scevgep408, i32 %tmp411 ; <i16*> [#uses=1]
+  store i16 undef, i16* %scevgep412, align 2
+  store i32 undef, i32* %scevgep407, align 4
+  %4 = add nsw i32 %index.6379, 1                 ; <i32> [#uses=1]
+  br label %bb169
+}





More information about the llvm-commits mailing list