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

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.
+      // value of N with the corresponding value of M.  So do that now.  However
-      N->setNodeId(ReadyToProcess);
+      // 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.
+        // Replacing the value takes care of remapping the new value.  Do the
-        ReplaceValueWith(SDValue(N, i), SDValue(M, i));
+        // replacement without recording it in ReplacedValues.  This does not
-      // Fall through.
+        // 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
+/// ReplaceValueWithHelper - Internal helper for ReplaceValueWith.  Updates the
-/// value.  Update the DAG and NodeIds replacing any uses of From to use To
+/// DAG causing any uses of From to use To instead, but without expunging From
-/// instead.
+/// or recording the replacement in ReplacedValues.  Do not call directly unless
-void DAGTypeLegalizer::ReplaceValueWith(SDValue From, SDValue To) {
+/// you really know what you are doing!
-  assert(From.getNode()->getNodeId() == ReadyToProcess &&
+void DAGTypeLegalizer::ReplaceValueWithHelper(SDValue From, SDValue To) {
-         "Only the node being processed may be remapped!");
   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);
 

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,

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
+}