From cc0a0cb4b76997ddb32386572d604ac216a66389 Mon Sep 17 00:00:00 2001 From: Duncan Sands Date: Fri, 20 Nov 2009 10:45:10 +0000 Subject: [PATCH] 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. llvm-svn: 89454 --- .../CodeGen/SelectionDAG/LegalizeTypes.cpp | 57 ++++++++----------- llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h | 1 - .../CodeGen/Generic/2009-11-20-NewNode.ll | 37 ++++++++++++ 3 files changed, 60 insertions(+), 35 deletions(-) create mode 100644 llvm/test/CodeGen/Generic/2009-11-20-NewNode.ll diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp index c4bd552f52ab..e2986493d4dc 100644 --- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp @@ -64,8 +64,12 @@ void DAGTypeLegalizer::PerformExpensiveChecks() { // 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 NewNodes; for (SelectionDAG::allnodes_iterator I = DAG.allnodes_begin(), E = DAG.allnodes_end(); I != E; ++I) { @@ -114,7 +118,11 @@ void DAGTypeLegalizer::PerformExpensiveChecks() { 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 @@ 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. 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 @@ namespace { } -/// 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 @@ void DAGTypeLegalizer::ReplaceValueWithHelper(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(); @@ -712,25 +720,6 @@ 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()->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"); diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h index e1b7022dda23..7b9b010e6301 100644 --- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h +++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h @@ -196,7 +196,6 @@ private: 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); diff --git a/llvm/test/CodeGen/Generic/2009-11-20-NewNode.ll b/llvm/test/CodeGen/Generic/2009-11-20-NewNode.ll new file mode 100644 index 000000000000..890f5c1f81b0 --- /dev/null +++ b/llvm/test/CodeGen/Generic/2009-11-20-NewNode.ll @@ -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 ; [#uses=3] + %conv69 = sext i16 %base to i64 ; [#uses=1] + %div = udiv i64 %cond66, %conv69 ; [#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 ; [#uses=1] + %or.cond = or i1 undef, %cmp123 ; [#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 +}