[llvm-commits] [llvm] r63048 - in /llvm/trunk: lib/CodeGen/SelectionDAG/LegalizeTypes.cpp lib/CodeGen/SelectionDAG/LegalizeTypes.h test/CodeGen/X86/2009-01-26-WrongCheck.ll

Duncan Sands baldrick at free.fr
Mon Jan 26 13:54:19 PST 2009


Author: baldrick
Date: Mon Jan 26 15:54:18 2009
New Revision: 63048

URL: http://llvm.org/viewvc/llvm-project?rev=63048&view=rev
Log:
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".

Added:
    llvm/trunk/test/CodeGen/X86/2009-01-26-WrongCheck.ll
Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
    llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp?rev=63048&r1=63047&r2=63048&view=diff

==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp Mon Jan 26 15:54:18 2009
@@ -328,14 +328,21 @@
         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 @@
 }
 
 
-/// 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 @@
   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 @@
   }
 }
 
+/// 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);
 

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h?rev=63048&r1=63047&r2=63048&view=diff

==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h Mon Jan 26 15:54:18 2009
@@ -199,6 +199,7 @@
                       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,

Added: llvm/trunk/test/CodeGen/X86/2009-01-26-WrongCheck.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2009-01-26-WrongCheck.ll?rev=63048&view=auto

==============================================================================
--- llvm/trunk/test/CodeGen/X86/2009-01-26-WrongCheck.ll (added)
+++ llvm/trunk/test/CodeGen/X86/2009-01-26-WrongCheck.ll Mon Jan 26 15:54:18 2009
@@ -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
+}





More information about the llvm-commits mailing list