Fix PR3393, which amounts to a bug in the expensive

checking logic.  Rather than make the checking more
complicated, I've tweaked some logic to make things
conform to how the checking thought things ought to
be, since this results in a simpler "mental model".

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

diff --git a/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp b/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
index 4c5dd030c7a11ee8652a0ac4ff652c22fb84f8df..36180c33d46ec68efd273a52400e44720e7b11f6 100644 (file)
--- a/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
+++ b/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
@@ -328,14 +328,21 @@ ScanOperands:
         continue;
 
       // The node morphed - this is equivalent to legalizing by replacing every
-      // value of N with the corresponding value of M.  So do that now.
-      N->setNodeId(ReadyToProcess);
+      // value of N with the corresponding value of M.  So do that now.  However
+      // there is no need to remember the replacement - morphing will make sure
+      // it is never used non-trivially.
       assert(N->getNumValues() == M->getNumValues() &&
              "Node morphing changed the number of results!");
       for (unsigned i = 0, e = N->getNumValues(); i != e; ++i)
-        // Replacing the value takes care of remapping the new value.
-        ReplaceValueWith(SDValue(N, i), SDValue(M, i));
-      // Fall through.
+        // Replacing the value takes care of remapping the new value.  Do the
+        // replacement without recording it in ReplacedValues.  This does not
+        // expunge From but that is fine - it is not really a new node.
+        ReplaceValueWithHelper(SDValue(N, i), SDValue(M, i));
+      assert(N->getNodeId() == NewNode && "Unexpected node state!");
+      // The node continues to live on as part of the NewNode fungus that
+      // grows on top of the useful nodes.  Nothing more needs to be done
+      // with it - move on to the next node.
+      continue;
     }
 
     if (i == NumOperands) {
@@ -668,16 +675,14 @@ namespace {
 }
 
 
-/// ReplaceValueWith - The specified value was legalized to the specified other
-/// value.  Update the DAG and NodeIds replacing any uses of From to use To
-/// instead.
-void DAGTypeLegalizer::ReplaceValueWith(SDValue From, SDValue To) {
-  assert(From.getNode()->getNodeId() == ReadyToProcess &&
-         "Only the node being processed may be remapped!");
+/// ReplaceValueWithHelper - Internal helper for ReplaceValueWith.  Updates the
+/// DAG causing any uses of From to use To instead, but without expunging From
+/// or recording the replacement in ReplacedValues.  Do not call directly unless
+/// you really know what you are doing!
+void DAGTypeLegalizer::ReplaceValueWithHelper(SDValue From, SDValue To) {
   assert(From.getNode() != To.getNode() && "Potential legalization loop!");
 
   // If expansion produced new nodes, make sure they are properly marked.
-  ExpungeNode(From.getNode());
   AnalyzeNewValue(To); // Expunges To.
 
   // Anything that used the old node should now use the new one.  Note that this
@@ -686,10 +691,6 @@ void DAGTypeLegalizer::ReplaceValueWith(SDValue From, SDValue To) {
   NodeUpdateListener NUL(*this, NodesToAnalyze);
   DAG.ReplaceAllUsesOfValueWith(From, To, &NUL);
 
-  // The old node may still be present in a map like ExpandedIntegers or
-  // PromotedIntegers.  Inform maps about the replacement.
-  ReplacedValues[From] = To;
-
   // Process the list of nodes that need to be reanalyzed.
   while (!NodesToAnalyze.empty()) {
     SDNode *N = NodesToAnalyze.back();
@@ -719,6 +720,25 @@ void DAGTypeLegalizer::ReplaceValueWith(SDValue From, SDValue To) {
   }
 }
 
+/// ReplaceValueWith - The specified value was legalized to the specified other
+/// value.  Update the DAG and NodeIds replacing any uses of From to use To
+/// instead.
+void DAGTypeLegalizer::ReplaceValueWith(SDValue From, SDValue To) {
+  assert(From.getNode()->getNodeId() == ReadyToProcess &&
+         "Only the node being processed may be remapped!");
+
+  // If expansion produced new nodes, make sure they are properly marked.
+  ExpungeNode(From.getNode());
+  AnalyzeNewValue(To); // Expunges To.
+
+  // The old node may still be present in a map like ExpandedIntegers or
+  // PromotedIntegers.  Inform maps about the replacement.
+  ReplacedValues[From] = To;
+
+  // Do the replacement.
+  ReplaceValueWithHelper(From, To);
+}
+
 void DAGTypeLegalizer::SetPromotedInteger(SDValue Op, SDValue Result) {
   AnalyzeNewValue(Result);
 

diff --git a/lib/CodeGen/SelectionDAG/LegalizeTypes.h b/lib/CodeGen/SelectionDAG/LegalizeTypes.h
index a2c91cb5bedbc9f683c2a1af7d039a6a0541330b..17b79233e49bf0d7ca241ac454b3ac87a196a6e6 100644 (file)
--- a/lib/CodeGen/SelectionDAG/LegalizeTypes.h
+++ b/lib/CodeGen/SelectionDAG/LegalizeTypes.h
@@ -199,6 +199,7 @@ private:
                       const SDValue *Ops, unsigned NumOps, bool isSigned);
   SDValue PromoteTargetBoolean(SDValue Bool, MVT VT);
   void ReplaceValueWith(SDValue From, SDValue To);
+  void ReplaceValueWithHelper(SDValue From, SDValue To);
   void SetIgnoredNodeResult(SDNode* N);
   void SplitInteger(SDValue Op, SDValue &Lo, SDValue &Hi);
   void SplitInteger(SDValue Op, MVT LoVT, MVT HiVT,

diff --git a/test/CodeGen/X86/2009-01-26-WrongCheck.ll b/test/CodeGen/X86/2009-01-26-WrongCheck.ll
new file mode 100644 (file)
index 0000000..db9dbb6
--- /dev/null
+++ b/test/CodeGen/X86/2009-01-26-WrongCheck.ll
@@ -0,0 +1,16 @@
+; RUN: llvm-as < %s | llc -march=x86 -enable-legalize-types-checking
+; PR3393
+
+define void @foo(i32 inreg %x) {
+       %t709 = select i1 false, i32 0, i32 %x          ; <i32> [#uses=1]
+       %t711 = add i32 %t709, 1                ; <i32> [#uses=4]
+       %t801 = icmp slt i32 %t711, 0           ; <i1> [#uses=1]
+       %t712 = zext i32 %t711 to i64           ; <i64> [#uses=1]
+       %t804 = select i1 %t801, i64 0, i64 %t712               ; <i64> [#uses=1]
+       store i64 %t804, i64* null
+       %t815 = icmp slt i32 %t711, 0           ; <i1> [#uses=1]
+       %t814 = sext i32 %t711 to i64           ; <i64> [#uses=1]
+       %t816 = select i1 %t815, i64 0, i64 %t814               ; <i64> [#uses=1]
+       store i64 %t816, i64* null
+       unreachable
+}