[llvm] r267099 - ValueMapper/Enumerator: Clean up code in post-order traversals, NFC
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 21 19:33:06 PDT 2016
Author: dexonsmith
Date: Thu Apr 21 21:33:06 2016
New Revision: 267099
URL: http://llvm.org/viewvc/llvm-project?rev=267099&view=rev
Log:
ValueMapper/Enumerator: Clean up code in post-order traversals, NFC
Re-layer the functions in the new (i.e., newly correct) post-order
traversals in ValueEnumerator (r266947) and ValueMapper (r266949).
Instead of adding a node to the worklist in a helper function and
returning a flag to say what happened, return the node itself. This
makes the code way cleaner: the worklist is local to the main function,
there is no flag for an early loop exit (since we can cleanly bury the
loop), and it's perfectly clear when pointers into the worklist might be
invalidated.
I'm fixing both algorithms in the same commit to avoid repeating the
commit message; if you take the time to understand one the other should
be easy. The diff itself isn't entirely obvious since the traversals
have some noise (i.e., things to do), but here's the high-level change:
auto helper = [&WL](T *Op) { auto helper = [](T **&I, T **E) {
=> while (I != E) {
if (shouldVisit(Op)) { T *Op = *I++;
WL.push(Op, Op->begin()); if (shouldVisit(Op)) {
return true; return Op;
} }
return false; return nullptr;
}; };
=>
WL.push(S, S->begin()); WL.push(S, S->begin());
while (!empty()) { while (!empty()) {
auto *N = WL.top().N; auto *N = WL.top().N;
auto *&I = WL.top().I; auto *&I = WL.top().I;
bool DidChange = false;
while (I != N->end())
if (helper(*I++)) { => if (T *Op = helper(I, N->end()) {
DidChange = true; WL.push(Op, Op->begin());
break; continue;
} }
if (DidChange)
continue;
POT.push(WL.pop()); => POT.push(WL.pop());
} }
Thanks to Mehdi for helping me find a better way to layer this.
Modified:
llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp
llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.h
llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
Modified: llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp?rev=267099&r1=267098&r2=267099&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp Thu Apr 21 21:33:06 2016
@@ -569,36 +569,40 @@ void ValueEnumerator::dropFunctionFromMe
void ValueEnumerator::EnumerateMetadata(unsigned F, const Metadata *MD) {
// Start by enumerating MD, and then work through its transitive operands in
// post-order. This requires a depth-first search.
- SmallVector<std::pair<const MDNode *, const MDOperand *>, 32> Worklist;
- enumerateMetadataImpl(F, MD, Worklist);
+ SmallVector<std::pair<const MDNode *, MDNode::op_iterator>, 32> Worklist;
+ if (const MDNode *N = enumerateMetadataImpl(F, MD))
+ Worklist.push_back(std::make_pair(N, N->op_begin()));
+
while (!Worklist.empty()) {
const MDNode *N = Worklist.back().first;
- const MDOperand *&Op = Worklist.back().second; // Be careful of lifetime...
+ MDNode::op_iterator &I = Worklist.back().second;
// Enumerate operands until the worklist changes. We need to traverse new
// nodes before visiting the rest of N's operands.
- bool DidWorklistChange = false;
- for (const MDOperand *E = N->op_end(); Op != E;)
- if (enumerateMetadataImpl(F, *Op++, Worklist)) {
- DidWorklistChange = true;
- break;
- }
- if (DidWorklistChange)
+ if (const MDNode *Op = enumerateMetadataOperands(F, I, N->op_end())) {
+ Worklist.push_back(std::make_pair(Op, Op->op_begin()));
continue;
+ }
// All the operands have been visited. Now assign an ID.
Worklist.pop_back();
MDs.push_back(N);
MetadataMap[N].ID = MDs.size();
- continue;
}
}
-bool ValueEnumerator::enumerateMetadataImpl(
- unsigned F, const Metadata *MD,
- SmallVectorImpl<std::pair<const MDNode *, const MDOperand *>> &Worklist) {
+const MDNode *
+ValueEnumerator::enumerateMetadataOperands(unsigned F, MDNode::op_iterator &I,
+ MDNode::op_iterator E) {
+ while (I != E)
+ if (const MDNode *N = enumerateMetadataImpl(F, *I++)) // Always increment I.
+ return N;
+ return nullptr;
+}
+
+const MDNode *ValueEnumerator::enumerateMetadataImpl(unsigned F, const Metadata *MD) {
if (!MD)
- return false;
+ return nullptr;
assert(
(isa<MDNode>(MD) || isa<MDString>(MD) || isa<ConstantAsMetadata>(MD)) &&
@@ -610,14 +614,12 @@ bool ValueEnumerator::enumerateMetadataI
// Already mapped. If F doesn't match the function tag, drop it.
if (Entry.hasDifferentFunction(F))
dropFunctionFromMetadata(*Insertion.first);
- return false;
+ return nullptr;
}
- // MDNodes are handled separately to avoid recursion.
- if (auto *N = dyn_cast<MDNode>(MD)) {
- Worklist.push_back(std::make_pair(N, N->op_begin()));
- return true; // Changed the worklist.
- }
+ // Don't assign IDs to metadata nodes.
+ if (auto *N = dyn_cast<MDNode>(MD))
+ return N;
// Save the metadata.
MDs.push_back(MD);
@@ -627,7 +629,7 @@ bool ValueEnumerator::enumerateMetadataI
if (auto *C = dyn_cast<ConstantAsMetadata>(MD))
EnumerateValue(C->getValue());
- return false;
+ return nullptr;
}
/// EnumerateFunctionLocalMetadataa - Incorporate function-local metadata
Modified: llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.h?rev=267099&r1=267098&r2=267099&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.h (original)
+++ llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.h Thu Apr 21 21:33:06 2016
@@ -246,9 +246,24 @@ private:
/// function.
void incorporateFunctionMetadata(const Function &F);
- bool enumerateMetadataImpl(
- unsigned F, const Metadata *MD,
- SmallVectorImpl<std::pair<const MDNode *, const MDOperand *>> &Worklist);
+ /// Enumerate operands with the given function tag.
+ ///
+ /// Enumerate the Metadata operands between \c I and \c E, returning the
+ /// first newly-enumerated MDNode without assigning it an ID.
+ ///
+ /// \post If a node was found, \c I points just past the node.
+ /// \post If no node was found, \c I is equal to \c E.
+ const MDNode *enumerateMetadataOperands(unsigned F, const MDOperand *&I,
+ const MDOperand *E);
+
+ /// Enumerate a single instance of metadata with the given function tag.
+ ///
+ /// If \c MD has already been enumerated, check that \c F matches its
+ /// function tag. If not, call \a dropFunctionFromMetadata().
+ ///
+ /// Otherwise, mark \c MD as visited. Assign it an ID, or just return it if
+ /// it's an \a MDNode.
+ const MDNode *enumerateMetadataImpl(unsigned F, const Metadata *MD);
unsigned getMetadataFunctionID(const Function *F) const;
void EnumerateMetadata(const Function *F, const Metadata *MD);
Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=267099&r1=267098&r2=267099&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Thu Apr 21 21:33:06 2016
@@ -228,18 +228,6 @@ class MDNodeMapper {
Metadata &getFwdReference(MDNode &Op);
};
- /// An entry in the worklist for the post-order traversal.
- struct POTWorklistEntry {
- MDNode *N; ///< Current node.
- const MDOperand *Op; ///< Current operand of \c N.
-
- /// Keep a flag of whether operands have changed in the worklist to avoid
- /// hitting the map in \a UniquedGraph.
- bool HasChanged = false;
-
- POTWorklistEntry(MDNode &N) : N(&N), Op(N.op_begin()) {}
- };
-
/// Worklist of distinct nodes whose operands need to be remapped.
SmallVector<MDNode *, 16> DistinctWorklist;
@@ -330,11 +318,15 @@ private:
/// to change because of operands outside the graph.
bool createPOT(UniquedGraph &G, const MDNode &FirstN);
- /// Visit an operand of a node in the POT.
+ /// Visit the operands of a uniqued node in the POT.
///
- /// Return \c true iff a new node was pushed onto \c Worklist.
- bool visitOperand(UniquedGraph &G, Metadata *Op,
- SmallVectorImpl<POTWorklistEntry> &Worklist);
+ /// Visit the operands in the range from \c I to \c E, returning the first
+ /// uniqued node we find that isn't yet in \c G. \c I is always advanced to
+ /// where to continue the loop through the operands.
+ ///
+ /// This sets \c HasChanged if any of the visited operands change.
+ MDNode *visitOperands(UniquedGraph &G, MDNode::op_iterator &I,
+ MDNode::op_iterator E, bool &HasChanged);
/// Map all the nodes in the given uniqued graph.
///
@@ -609,6 +601,20 @@ void MDNodeMapper::remapOperands(MDNode
}
}
+namespace {
+/// An entry in the worklist for the post-order traversal.
+struct POTWorklistEntry {
+ MDNode *N; ///< Current node.
+ MDNode::op_iterator Op; ///< Current operand of \c N.
+
+ /// Keep a flag of whether operands have changed in the worklist to avoid
+ /// hitting the map in \a UniquedGraph.
+ bool HasChanged = false;
+
+ POTWorklistEntry(MDNode &N) : N(&N), Op(N.op_begin()) {}
+};
+} // end namespace
+
bool MDNodeMapper::createPOT(UniquedGraph &G, const MDNode &FirstN) {
assert(G.Info.empty() && "Expected a fresh traversal");
assert(FirstN.isUniqued() && "Expected uniqued node in POT");
@@ -619,49 +625,46 @@ bool MDNodeMapper::createPOT(UniquedGrap
Worklist.push_back(POTWorklistEntry(const_cast<MDNode &>(FirstN)));
(void)G.Info[&FirstN];
while (!Worklist.empty()) {
- MDNode &N = *Worklist.back().N;
- const MDOperand *&Op = Worklist.back().Op; // Careful of lifetime...
- assert(N.isUniqued() && "Expected only uniqued nodes in POT");
-
- // Pick up the traversal from Op and continue. Since this is a DFS, pause
- // as soon as a new node is pushed onto the worklist.
- bool DidWorklistSizeChange = false;
- for (const MDOperand *E = N.op_end(); Op != E;) {
- assert(*Op != &N && "Uniqued nodes cannot have self-references");
- if (visitOperand(G, *Op++, Worklist)) {
- DidWorklistSizeChange = true;
- break;
- }
- }
- if (DidWorklistSizeChange)
+ // Start or continue the traversal through the this node's operands.
+ auto &WE = Worklist.back();
+ if (MDNode *N = visitOperands(G, WE.Op, WE.N->op_end(), WE.HasChanged)) {
+ // Push a new node to traverse first.
+ Worklist.push_back(POTWorklistEntry(*N));
continue;
+ }
- // All operands of N have been visited. Push N into the POT.
- auto &D = G.Info[&N];
- AnyChanges |= D.HasChanged = Worklist.pop_back_val().HasChanged;
+ // Push the node onto the POT.
+ assert(WE.N->isUniqued() && "Expected only uniqued nodes");
+ assert(WE.Op == WE.N->op_end() && "Expected to visit all operands");
+ auto &D = G.Info[WE.N];
+ AnyChanges |= D.HasChanged = WE.HasChanged;
D.ID = G.POT.size();
- G.POT.push_back(&N);
+ G.POT.push_back(WE.N);
+
+ // Pop the node off the worklist.
+ Worklist.pop_back();
}
return AnyChanges;
}
-bool MDNodeMapper::visitOperand(UniquedGraph &G, Metadata *Op,
- SmallVectorImpl<POTWorklistEntry> &Worklist) {
- // Try to map Op, and check it for changes.
- if (Optional<Metadata *> MappedOp = tryToMapOperand(Op)) {
- Worklist.back().HasChanged |= Op != *MappedOp;
- return false;
- }
-
- // Push Op onto the Worklist unless it's already in G.
- MDNode &OpN = *cast<MDNode>(Op);
- assert(OpN.isUniqued() &&
- "Only uniqued operands cannot be mapped immediately");
- if (!G.Info.insert(std::make_pair(&OpN, Data())).second)
- return false;
+MDNode *MDNodeMapper::visitOperands(UniquedGraph &G, MDNode::op_iterator &I,
+ MDNode::op_iterator E, bool &HasChanged) {
+ while (I != E) {
+ Metadata *Op = *I++; // Increment even on early return.
+ if (Optional<Metadata *> MappedOp = tryToMapOperand(Op)) {
+ // Check if the operand changes.
+ HasChanged |= Op != *MappedOp;
+ continue;
+ }
- Worklist.push_back(POTWorklistEntry(OpN));
- return true;
+ // A uniqued metadata node.
+ MDNode &OpN = *cast<MDNode>(Op);
+ assert(OpN.isUniqued() &&
+ "Only uniqued operands cannot be mapped immediately");
+ if (G.Info.insert(std::make_pair(&OpN, Data())).second)
+ return &OpN; // This is a new one. Return it.
+ }
+ return nullptr;
}
void MDNodeMapper::UniquedGraph::propagateChanges() {
More information about the llvm-commits
mailing list