[llvm-commits] [llvm] r89454 - in /llvm/trunk: lib/CodeGen/SelectionDAG/LegalizeTypes.cpp lib/CodeGen/SelectionDAG/LegalizeTypes.h test/CodeGen/Generic/2009-11-20-NewNode.ll

Duncan Sands baldrick at free.fr
Fri Nov 20 02:45:11 PST 2009


Author: baldrick
Date: Fri Nov 20 04:45:10 2009
New Revision: 89454

URL: http://llvm.org/viewvc/llvm-project?rev=89454&view=rev
Log:
Fix PR5558, which was caused by a wrong fix for PR3393 (see commit 63048),
which was an expensive checks failure due to a bug in the checking.  This
patch in essence reverts the original fix for PR3393, and refixes it by a
tweak to the way expensive checking is done.

Added:
    llvm/trunk/test/CodeGen/Generic/2009-11-20-NewNode.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=89454&r1=89453&r2=89454&view=diff

==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp Fri Nov 20 04:45:10 2009
@@ -64,8 +64,12 @@
   // The final node obtained by mapping by ReplacedValues is not marked NewNode.
   // Note that ReplacedValues should be applied iteratively.
 
-  // Note that the ReplacedValues map may also map deleted nodes.  By iterating
-  // over the DAG we only consider non-deleted nodes.
+  // Note that the ReplacedValues map may also map deleted nodes (by iterating
+  // over the DAG we never dereference deleted nodes).  This means that it may
+  // also map nodes marked NewNode if the deallocated memory was reallocated as
+  // another node, and that new node was not seen by the LegalizeTypes machinery
+  // (for example because it was created but not used).  In general, we cannot
+  // distinguish between new nodes and deleted nodes.
   SmallVector<SDNode*, 16> NewNodes;
   for (SelectionDAG::allnodes_iterator I = DAG.allnodes_begin(),
        E = DAG.allnodes_end(); I != E; ++I) {
@@ -114,7 +118,11 @@
         Mapped |= 128;
 
       if (I->getNodeId() != Processed) {
-        if (Mapped != 0) {
+        // Since we allow ReplacedValues to map deleted nodes, it may map nodes
+        // marked NewNode too, since a deleted node may have been reallocated as
+        // another node that has not been seen by the LegalizeTypes machinery.
+        if ((I->getNodeId() == NewNode && Mapped > 1) ||
+            (I->getNodeId() != NewNode && Mapped != 0)) {
           errs() << "Unprocessed value in a map!";
           Failed = true;
         }
@@ -320,16 +328,12 @@
         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.  However
-      // there is no need to remember the replacement - morphing will make sure
-      // it is never used non-trivially.
+      // value of N with the corresponding value of M.  So do that now.
       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.  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));
+        // Replacing the value takes care of remapping the new value.
+        ReplaceValueWith(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
@@ -666,14 +670,14 @@
 }
 
 
-/// 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) {
+/// 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() != 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
@@ -682,6 +686,10 @@
   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();
@@ -712,25 +720,6 @@
   }
 }
 
-/// 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) {
   assert(Result.getValueType() == TLI.getTypeToTransformTo(*DAG.getContext(), Op.getValueType()) &&
          "Invalid type for promoted integer");

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

==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h Fri Nov 20 04:45:10 2009
@@ -196,7 +196,6 @@
                       DebugLoc dl);
   SDValue PromoteTargetBoolean(SDValue Bool, EVT VT);
   void ReplaceValueWith(SDValue From, SDValue To);
-  void ReplaceValueWithHelper(SDValue From, SDValue To);
   void SplitInteger(SDValue Op, SDValue &Lo, SDValue &Hi);
   void SplitInteger(SDValue Op, EVT LoVT, EVT HiVT,
                     SDValue &Lo, SDValue &Hi);

Added: llvm/trunk/test/CodeGen/Generic/2009-11-20-NewNode.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Generic/2009-11-20-NewNode.ll?rev=89454&view=auto

==============================================================================
--- llvm/trunk/test/CodeGen/Generic/2009-11-20-NewNode.ll (added)
+++ llvm/trunk/test/CodeGen/Generic/2009-11-20-NewNode.ll Fri Nov 20 04:45:10 2009
@@ -0,0 +1,37 @@
+; RUN: llc -march=msp430 %s
+; RUN: llc -march=pic16 %s
+; PR5558
+
+define i64 @_strtoll_r(i16 %base) nounwind {
+entry:
+  br i1 undef, label %if.then, label %if.end27
+
+if.then:                                          ; preds = %do.end
+  br label %if.end27
+
+if.end27:                                         ; preds = %if.then, %do.end
+  %cond66 = select i1 undef, i64 -9223372036854775808, i64 9223372036854775807 ; <i64> [#uses=3]
+  %conv69 = sext i16 %base to i64                 ; <i64> [#uses=1]
+  %div = udiv i64 %cond66, %conv69                ; <i64> [#uses=1]
+  br label %for.cond
+
+for.cond:                                         ; preds = %if.end116, %if.end27
+  br i1 undef, label %if.then152, label %if.then93
+
+if.then93:                                        ; preds = %for.cond
+  br i1 undef, label %if.end116, label %if.then152
+
+if.end116:                                        ; preds = %if.then93
+  %cmp123 = icmp ugt i64 undef, %div              ; <i1> [#uses=1]
+  %or.cond = or i1 undef, %cmp123                 ; <i1> [#uses=0]
+  br label %for.cond
+
+if.then152:                                       ; preds = %if.then93, %for.cond
+  br i1 undef, label %if.end182, label %if.then172
+
+if.then172:                                       ; preds = %if.then152
+  ret i64 %cond66
+
+if.end182:                                        ; preds = %if.then152
+  ret i64 %cond66
+}





More information about the llvm-commits mailing list