[llvm-commits] [llvm] r58372 - in /llvm/trunk/lib/CodeGen/SelectionDAG: LegalizeTypes.cpp LegalizeTypes.h
Duncan Sands
baldrick at free.fr
Tue Oct 28 23:42:19 PDT 2008
Author: baldrick
Date: Wed Oct 29 01:42:19 2008
New Revision: 58372
URL: http://llvm.org/viewvc/llvm-project?rev=58372&view=rev
Log:
Fix a FIXME: in ReplaceNodeWith, if the new node
is morphed by AnalyzeNewNode into a previously
processed node, and different result values of
that node are remapped to values with different
nodes, then we could end up using wrong values
here [we were assuming that all results remap
to values with the same underlying node]. This
seems theoretically possible, but I don't have
a testcase. The meat of the patch is in the
changes to AnalyzeNewNode/AnalyzeNewValue and
ReplaceNodeWith. While there, I changed names
like RemapNode to RemapValue, since it really
remaps values. To tell the truth, I would be
much happier if we were only remapping nodes
(it would simplify a bunch of logic, and allow
for some cute speedups) but I haven't yet worked
out how to do that.
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=58372&r1=58371&r2=58372&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp Wed Oct 29 01:42:19 2008
@@ -220,14 +220,13 @@
/// AnalyzeNewNode - The specified node is the root of a subtree of potentially
/// new nodes. Correct any processed operands (this may change the node) and
-/// calculate the NodeId.
+/// calculate the NodeId. If the node itself changes to a processed node, it
+/// is not remapped - the caller needs to take care of this.
/// Returns the potentially changed node.
-void DAGTypeLegalizer::AnalyzeNewNode(SDValue &Val) {
- SDNode *N = Val.getNode();
-
+SDNode *DAGTypeLegalizer::AnalyzeNewNode(SDNode *N) {
// If this was an existing node that is already done, we're done.
if (N->getNodeId() != NewNode)
- return;
+ return N;
// Remove any stale map entries.
ExpungeNode(N);
@@ -250,9 +249,9 @@
SDValue Op = OrigOp;
if (Op.getNode()->getNodeId() == Processed)
- RemapNode(Op);
+ RemapValue(Op);
else if (Op.getNode()->getNodeId() == NewNode)
- AnalyzeNewNode(Op);
+ AnalyzeNewValue(Op);
if (Op.getNode()->getNodeId() == Processed)
++NumProcessed;
@@ -270,21 +269,16 @@
// Some operands changed - update the node.
if (!NewOps.empty()) {
- Val = DAG.UpdateNodeOperands(Val, &NewOps[0], NewOps.size());
- if (Val.getNode() != N) {
- // The node morphed, work with the new node.
- N = Val.getNode();
-
- // Maybe it morphed into a previously analyzed node?
- if (N->getNodeId() != NewNode) {
- if (N->getNodeId() == Processed)
- // An already processed node may need to be remapped.
- RemapNode(Val);
- return;
- }
+ SDNode *M = DAG.UpdateNodeOperands(SDValue(N, 0), &NewOps[0],
+ NewOps.size()).getNode();
+ if (M != N) {
+ if (M->getNodeId() != NewNode)
+ // It morphed into a previously analyzed node - nothing more to do.
+ return M;
// It morphed into a different new node. Do the equivalent of passing
// it to AnalyzeNewNode: expunge it and calculate the NodeId.
+ N = M;
ExpungeNode(N);
}
}
@@ -293,6 +287,23 @@
N->setNodeId(N->getNumOperands()-NumProcessed);
if (N->getNodeId() == ReadyToProcess)
Worklist.push_back(N);
+
+ return N;
+}
+
+/// AnalyzeNewValue - Call AnalyzeNewNode, updating the node in Val if needed.
+/// If the node changes to a processed node, then remap it.
+void DAGTypeLegalizer::AnalyzeNewValue(SDValue &Val) {
+ SDNode *N(Val.getNode());
+ // If this was an existing node that is already done, avoid remapping it.
+ if (N->getNodeId() != NewNode)
+ return;
+ SDNode *M(AnalyzeNewNode(N));
+ if (M != N)
+ Val.setNode(M);
+ if (M->getNodeId() == Processed)
+ // It morphed into an already processed node - remap it.
+ RemapValue(Val);
}
@@ -310,7 +321,7 @@
N->getNodeId() != DAGTypeLegalizer::ReadyToProcess &&
"RAUW deleted processed node!");
// It is possible, though rare, for the deleted node N to occur as a
- // target in a map, so note the replacement N -> E in ReplacedNodes.
+ // target in a map, so note the replacement N -> E in ReplacedValues.
assert(E && "Node not replaced?");
DTL.NoteDeletion(N, E);
}
@@ -336,7 +347,7 @@
// If expansion produced new nodes, make sure they are properly marked.
ExpungeNode(From.getNode());
- AnalyzeNewNode(To); // Expunges To.
+ AnalyzeNewValue(To); // Expunges To.
// Anything that used the old node should now use the new one. Note that this
// can potentially cause recursive merging.
@@ -345,7 +356,7 @@
// The old node may still be present in a map like ExpandedIntegers or
// PromotedIntegers. Inform maps about the replacement.
- ReplacedNodes[From] = To;
+ ReplacedValues[From] = To;
}
/// ReplaceNodeWith - Replace uses of the 'from' node's results with the 'to'
@@ -356,9 +367,9 @@
// If expansion produced new nodes, make sure they are properly marked.
ExpungeNode(From);
- SDValue Val(To, 0);
- AnalyzeNewNode(Val); // Expunges To. FIXME: All results mapped the same?
- To = Val.getNode();
+ To = AnalyzeNewNode(To); // Expunges To.
+ // If To morphed into an already processed node, its values may need
+ // remapping. This is done below.
assert(From->getNumValues() == To->getNumValues() &&
"Node results don't match");
@@ -366,51 +377,61 @@
// Anything that used the old node should now use the new one. Note that this
// can potentially cause recursive merging.
NodeUpdateListener NUL(*this);
- DAG.ReplaceAllUsesWith(From, To, &NUL);
-
- // The old node may still be present in a map like ExpandedIntegers or
- // PromotedIntegers. Inform maps about the replacement.
for (unsigned i = 0, e = From->getNumValues(); i != e; ++i) {
- assert(From->getValueType(i) == To->getValueType(i) &&
- "Node results don't match");
- ReplacedNodes[SDValue(From, i)] = SDValue(To, i);
+ SDValue FromVal(From, i);
+ SDValue ToVal(To, i);
+
+ // AnalyzeNewNode may have morphed a new node into a processed node. Remap
+ // values now.
+ if (To->getNodeId() == Processed)
+ RemapValue(ToVal);
+
+ assert(FromVal.getValueType() == ToVal.getValueType() &&
+ "Node results don't match!");
+
+ // Make anything that used the old value use the new value.
+ DAG.ReplaceAllUsesOfValueWith(FromVal, ToVal, &NUL);
+
+ // The old node may still be present in a map like ExpandedIntegers or
+ // PromotedIntegers. Inform maps about the replacement.
+ ReplacedValues[FromVal] = ToVal;
}
}
-/// RemapNode - If the specified value was already legalized to another value,
+/// RemapValue - If the specified value was already legalized to another value,
/// replace it by that value.
-void DAGTypeLegalizer::RemapNode(SDValue &N) {
- DenseMap<SDValue, SDValue>::iterator I = ReplacedNodes.find(N);
- if (I != ReplacedNodes.end()) {
+void DAGTypeLegalizer::RemapValue(SDValue &N) {
+ DenseMap<SDValue, SDValue>::iterator I = ReplacedValues.find(N);
+ if (I != ReplacedValues.end()) {
// Use path compression to speed up future lookups if values get multiply
// replaced with other values.
- RemapNode(I->second);
+ RemapValue(I->second);
N = I->second;
}
assert(N.getNode()->getNodeId() != NewNode && "Mapped to unanalyzed node!");
}
-/// ExpungeNode - If N has a bogus mapping in ReplacedNodes, eliminate it.
+/// ExpungeNode - If N has a bogus mapping in ReplacedValues, eliminate it.
/// This can occur when a node is deleted then reallocated as a new node -
-/// the mapping in ReplacedNodes applies to the deleted node, not the new
+/// the mapping in ReplacedValues applies to the deleted node, not the new
/// one.
-/// The only map that can have a deleted node as a source is ReplacedNodes.
+/// The only map that can have a deleted node as a source is ReplacedValues.
/// Other maps can have deleted nodes as targets, but since their looked-up
-/// values are always immediately remapped using RemapNode, resulting in a
-/// not-deleted node, this is harmless as long as ReplacedNodes/RemapNode
+/// values are always immediately remapped using RemapValue, resulting in a
+/// not-deleted node, this is harmless as long as ReplacedValues/RemapValue
/// always performs correct mappings. In order to keep the mapping correct,
/// ExpungeNode should be called on any new nodes *before* adding them as
-/// either source or target to ReplacedNodes (which typically means calling
+/// either source or target to ReplacedValues (which typically means calling
/// Expunge when a new node is first seen, since it may no longer be marked
-/// NewNode by the time it is added to ReplacedNodes).
+/// NewNode by the time it is added to ReplacedValues).
void DAGTypeLegalizer::ExpungeNode(SDNode *N) {
if (N->getNodeId() != NewNode)
return;
- // If N is not remapped by ReplacedNodes then there is nothing to do.
+ // If N is not remapped by ReplacedValues then there is nothing to do.
unsigned i, e;
for (i = 0, e = N->getNumValues(); i != e; ++i)
- if (ReplacedNodes.find(SDValue(N, i)) != ReplacedNodes.end())
+ if (ReplacedValues.find(SDValue(N, i)) != ReplacedValues.end())
break;
if (i == e)
@@ -421,52 +442,52 @@
for (DenseMap<SDValue, SDValue>::iterator I = PromotedIntegers.begin(),
E = PromotedIntegers.end(); I != E; ++I) {
assert(I->first.getNode() != N);
- RemapNode(I->second);
+ RemapValue(I->second);
}
for (DenseMap<SDValue, SDValue>::iterator I = SoftenedFloats.begin(),
E = SoftenedFloats.end(); I != E; ++I) {
assert(I->first.getNode() != N);
- RemapNode(I->second);
+ RemapValue(I->second);
}
for (DenseMap<SDValue, SDValue>::iterator I = ScalarizedVectors.begin(),
E = ScalarizedVectors.end(); I != E; ++I) {
assert(I->first.getNode() != N);
- RemapNode(I->second);
+ RemapValue(I->second);
}
for (DenseMap<SDValue, std::pair<SDValue, SDValue> >::iterator
I = ExpandedIntegers.begin(), E = ExpandedIntegers.end(); I != E; ++I){
assert(I->first.getNode() != N);
- RemapNode(I->second.first);
- RemapNode(I->second.second);
+ RemapValue(I->second.first);
+ RemapValue(I->second.second);
}
for (DenseMap<SDValue, std::pair<SDValue, SDValue> >::iterator
I = ExpandedFloats.begin(), E = ExpandedFloats.end(); I != E; ++I) {
assert(I->first.getNode() != N);
- RemapNode(I->second.first);
- RemapNode(I->second.second);
+ RemapValue(I->second.first);
+ RemapValue(I->second.second);
}
for (DenseMap<SDValue, std::pair<SDValue, SDValue> >::iterator
I = SplitVectors.begin(), E = SplitVectors.end(); I != E; ++I) {
assert(I->first.getNode() != N);
- RemapNode(I->second.first);
- RemapNode(I->second.second);
+ RemapValue(I->second.first);
+ RemapValue(I->second.second);
}
- for (DenseMap<SDValue, SDValue>::iterator I = ReplacedNodes.begin(),
- E = ReplacedNodes.end(); I != E; ++I)
- RemapNode(I->second);
+ for (DenseMap<SDValue, SDValue>::iterator I = ReplacedValues.begin(),
+ E = ReplacedValues.end(); I != E; ++I)
+ RemapValue(I->second);
for (unsigned i = 0, e = N->getNumValues(); i != e; ++i)
- ReplacedNodes.erase(SDValue(N, i));
+ ReplacedValues.erase(SDValue(N, i));
}
void DAGTypeLegalizer::SetPromotedInteger(SDValue Op, SDValue Result) {
- AnalyzeNewNode(Result);
+ AnalyzeNewValue(Result);
SDValue &OpEntry = PromotedIntegers[Op];
assert(OpEntry.getNode() == 0 && "Node is already promoted!");
@@ -474,7 +495,7 @@
}
void DAGTypeLegalizer::SetSoftenedFloat(SDValue Op, SDValue Result) {
- AnalyzeNewNode(Result);
+ AnalyzeNewValue(Result);
SDValue &OpEntry = SoftenedFloats[Op];
assert(OpEntry.getNode() == 0 && "Node is already converted to integer!");
@@ -482,7 +503,7 @@
}
void DAGTypeLegalizer::SetScalarizedVector(SDValue Op, SDValue Result) {
- AnalyzeNewNode(Result);
+ AnalyzeNewValue(Result);
SDValue &OpEntry = ScalarizedVectors[Op];
assert(OpEntry.getNode() == 0 && "Node is already scalarized!");
@@ -492,8 +513,8 @@
void DAGTypeLegalizer::GetExpandedInteger(SDValue Op, SDValue &Lo,
SDValue &Hi) {
std::pair<SDValue, SDValue> &Entry = ExpandedIntegers[Op];
- RemapNode(Entry.first);
- RemapNode(Entry.second);
+ RemapValue(Entry.first);
+ RemapValue(Entry.second);
assert(Entry.first.getNode() && "Operand isn't expanded");
Lo = Entry.first;
Hi = Entry.second;
@@ -502,8 +523,8 @@
void DAGTypeLegalizer::SetExpandedInteger(SDValue Op, SDValue Lo,
SDValue Hi) {
// Lo/Hi may have been newly allocated, if so, add nodeid's as relevant.
- AnalyzeNewNode(Lo);
- AnalyzeNewNode(Hi);
+ AnalyzeNewValue(Lo);
+ AnalyzeNewValue(Hi);
// Remember that this is the result of the node.
std::pair<SDValue, SDValue> &Entry = ExpandedIntegers[Op];
@@ -515,8 +536,8 @@
void DAGTypeLegalizer::GetExpandedFloat(SDValue Op, SDValue &Lo,
SDValue &Hi) {
std::pair<SDValue, SDValue> &Entry = ExpandedFloats[Op];
- RemapNode(Entry.first);
- RemapNode(Entry.second);
+ RemapValue(Entry.first);
+ RemapValue(Entry.second);
assert(Entry.first.getNode() && "Operand isn't expanded");
Lo = Entry.first;
Hi = Entry.second;
@@ -525,8 +546,8 @@
void DAGTypeLegalizer::SetExpandedFloat(SDValue Op, SDValue Lo,
SDValue Hi) {
// Lo/Hi may have been newly allocated, if so, add nodeid's as relevant.
- AnalyzeNewNode(Lo);
- AnalyzeNewNode(Hi);
+ AnalyzeNewValue(Lo);
+ AnalyzeNewValue(Hi);
// Remember that this is the result of the node.
std::pair<SDValue, SDValue> &Entry = ExpandedFloats[Op];
@@ -538,8 +559,8 @@
void DAGTypeLegalizer::GetSplitVector(SDValue Op, SDValue &Lo,
SDValue &Hi) {
std::pair<SDValue, SDValue> &Entry = SplitVectors[Op];
- RemapNode(Entry.first);
- RemapNode(Entry.second);
+ RemapValue(Entry.first);
+ RemapValue(Entry.second);
assert(Entry.first.getNode() && "Operand isn't split");
Lo = Entry.first;
Hi = Entry.second;
@@ -548,8 +569,8 @@
void DAGTypeLegalizer::SetSplitVector(SDValue Op, SDValue Lo,
SDValue Hi) {
// Lo/Hi may have been newly allocated, if so, add nodeid's as relevant.
- AnalyzeNewNode(Lo);
- AnalyzeNewNode(Hi);
+ AnalyzeNewValue(Lo);
+ AnalyzeNewValue(Hi);
// Remember that this is the result of the node.
std::pair<SDValue, SDValue> &Entry = SplitVectors[Op];
@@ -610,8 +631,8 @@
Hi = DAG.getNode(ISD::TRUNCATE, HiVT, Hi);
}
-/// SplitInteger - Return the lower and upper halves of Op's bits in a value type
-/// half the size of Op's.
+/// SplitInteger - Return the lower and upper halves of Op's bits in a value
+/// type half the size of Op's.
void DAGTypeLegalizer::SplitInteger(SDValue Op,
SDValue &Lo, SDValue &Hi) {
MVT HalfVT = MVT::getIntegerVT(Op.getValueType().getSizeInBits()/2);
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h?rev=58372&r1=58371&r2=58372&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h Wed Oct 29 01:42:19 2008
@@ -134,9 +134,9 @@
/// which operands are the expanded version of the input.
DenseMap<SDValue, std::pair<SDValue, SDValue> > SplitVectors;
- /// ReplacedNodes - For nodes that have been replaced with another,
- /// indicates the replacement node to use.
- DenseMap<SDValue, SDValue> ReplacedNodes;
+ /// ReplacedValues - For values that have been replaced with another,
+ /// indicates the replacement value to use.
+ DenseMap<SDValue, SDValue> ReplacedValues;
/// Worklist - This defines a worklist of nodes to process. In order to be
/// pushed onto this worklist, all operands of a node must have already been
@@ -157,8 +157,7 @@
/// for the specified node, adding it to the worklist if ready.
void ReanalyzeNode(SDNode *N) {
N->setNodeId(NewNode);
- SDValue Val(N, 0);
- AnalyzeNewNode(Val);
+ AnalyzeNewNode(N);
// The node may have changed but we don't care.
}
@@ -166,16 +165,17 @@
ExpungeNode(Old);
ExpungeNode(New);
for (unsigned i = 0, e = Old->getNumValues(); i != e; ++i)
- ReplacedNodes[SDValue(Old, i)] = SDValue(New, i);
+ ReplacedValues[SDValue(Old, i)] = SDValue(New, i);
}
private:
- void AnalyzeNewNode(SDValue &Val);
+ SDNode *AnalyzeNewNode(SDNode *N);
+ void AnalyzeNewValue(SDValue &Val);
void ReplaceValueWith(SDValue From, SDValue To);
void ReplaceNodeWith(SDNode *From, SDNode *To);
- void RemapNode(SDValue &N);
+ void RemapValue(SDValue &N);
void ExpungeNode(SDNode *N);
// Common routines.
@@ -197,7 +197,7 @@
SDValue GetPromotedInteger(SDValue Op) {
SDValue &PromotedOp = PromotedIntegers[Op];
- RemapNode(PromotedOp);
+ RemapValue(PromotedOp);
assert(PromotedOp.getNode() && "Operand wasn't promoted?");
return PromotedOp;
}
@@ -326,7 +326,7 @@
SDValue GetSoftenedFloat(SDValue Op) {
SDValue &SoftenedOp = SoftenedFloats[Op];
- RemapNode(SoftenedOp);
+ RemapValue(SoftenedOp);
assert(SoftenedOp.getNode() && "Operand wasn't converted to integer?");
return SoftenedOp;
}
@@ -406,7 +406,7 @@
SDValue GetScalarizedVector(SDValue Op) {
SDValue &ScalarizedOp = ScalarizedVectors[Op];
- RemapNode(ScalarizedOp);
+ RemapValue(ScalarizedOp);
assert(ScalarizedOp.getNode() && "Operand wasn't scalarized?");
return ScalarizedOp;
}
More information about the llvm-commits
mailing list