[llvm] r266947 - BitcodeWriter: Emit metadata in post-order (again)
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 20 18:55:13 PDT 2016
Author: dexonsmith
Date: Wed Apr 20 20:55:12 2016
New Revision: 266947
URL: http://llvm.org/viewvc/llvm-project?rev=266947&view=rev
Log:
BitcodeWriter: Emit metadata in post-order (again)
Emit metadata nodes in post-order. The iterative algorithm from r266709
failed to maintain this property. After understanding my mistake, it
wasn't too hard to write a test with llvm-bcanalyzer (and I've actually
made this change once before: see r220340).
This also reverts the "noisy" testcase change from r266709. That should
have been more of a red flag :/.
Note: The same bug crept into the ValueMapper in r265456. I'm still
working on the fix.
Added:
llvm/trunk/test/Bitcode/mdnodes-in-post-order.ll
Modified:
llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp
llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.h
llvm/trunk/test/Bitcode/metadata-function-blocks.ll
Modified: llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp?rev=266947&r1=266946&r2=266947&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp Wed Apr 20 20:55:12 2016
@@ -568,19 +568,23 @@ 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.
- SmallVector<std::pair<const MDNode *, bool>, 32> Worklist;
+ // post-order. This requires a depth-first search.
+ SmallVector<std::pair<const MDNode *, const MDOperand *>, 32> Worklist;
enumerateMetadataImpl(F, MD, Worklist);
while (!Worklist.empty()) {
const MDNode *N = Worklist.back().first;
- if (!Worklist.back().second) {
- // On the first visit, add the operands to the worklist.
- Worklist.back().second = true;
- unsigned F = MetadataMap.lookup(N).F;
- for (const Metadata *Op : N->operands())
- enumerateMetadataImpl(F, Op, Worklist);
+ const MDOperand *&Op = Worklist.back().second; // Be careful of lifetime...
+
+ // 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)
continue;
- }
// All the operands have been visited. Now assign an ID.
Worklist.pop_back();
@@ -590,11 +594,11 @@ void ValueEnumerator::EnumerateMetadata(
}
}
-void ValueEnumerator::enumerateMetadataImpl(
+bool ValueEnumerator::enumerateMetadataImpl(
unsigned F, const Metadata *MD,
- SmallVectorImpl<std::pair<const MDNode *, bool>> &Worklist) {
+ SmallVectorImpl<std::pair<const MDNode *, const MDOperand *>> &Worklist) {
if (!MD)
- return;
+ return false;
assert(
(isa<MDNode>(MD) || isa<MDString>(MD) || isa<ConstantAsMetadata>(MD)) &&
@@ -606,13 +610,13 @@ void ValueEnumerator::enumerateMetadataI
// Already mapped. If F doesn't match the function tag, drop it.
if (Entry.hasDifferentFunction(F))
dropFunctionFromMetadata(*Insertion.first);
- return;
+ return false;
}
// MDNodes are handled separately to avoid recursion.
if (auto *N = dyn_cast<MDNode>(MD)) {
- Worklist.push_back(std::make_pair(N, false));
- return;
+ Worklist.push_back(std::make_pair(N, N->op_begin()));
+ return true; // Changed the worklist.
}
// Save the metadata.
@@ -622,6 +626,8 @@ void ValueEnumerator::enumerateMetadataI
// Enumerate the constant, if any.
if (auto *C = dyn_cast<ConstantAsMetadata>(MD))
EnumerateValue(C->getValue());
+
+ return false;
}
/// 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=266947&r1=266946&r2=266947&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.h (original)
+++ llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.h Wed Apr 20 20:55:12 2016
@@ -34,6 +34,7 @@ class Module;
class Metadata;
class LocalAsMetadata;
class MDNode;
+class MDOperand;
class NamedMDNode;
class AttributeSet;
class ValueSymbolTable;
@@ -245,9 +246,9 @@ private:
/// function.
void incorporateFunctionMetadata(const Function &F);
- void enumerateMetadataImpl(
+ bool enumerateMetadataImpl(
unsigned F, const Metadata *MD,
- SmallVectorImpl<std::pair<const MDNode *, bool>> &Worklist);
+ SmallVectorImpl<std::pair<const MDNode *, const MDOperand *>> &Worklist);
unsigned getMetadataFunctionID(const Function *F) const;
void EnumerateMetadata(const Function *F, const Metadata *MD);
Added: llvm/trunk/test/Bitcode/mdnodes-in-post-order.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/mdnodes-in-post-order.ll?rev=266947&view=auto
==============================================================================
--- llvm/trunk/test/Bitcode/mdnodes-in-post-order.ll (added)
+++ llvm/trunk/test/Bitcode/mdnodes-in-post-order.ll Wed Apr 20 20:55:12 2016
@@ -0,0 +1,34 @@
+; RUN: llvm-as <%s | llvm-bcanalyzer -dump | FileCheck %s
+; Check that nodes are emitted in post-order to minimize the need for temporary
+; nodes. The graph structure is designed to foil naive implementations of
+; iteratitive post-order traersals: the leaves, !3 and !4, are reachable from
+; the entry node, !6, as well as from !5. There is one leaf on either side to
+; be sure it tickles bugs whether operands are visited forward or reverse.
+
+; Nodes in this testcase are numbered to match how they are referenced in
+; bitcode. !3 is referenced as opN=3.
+
+; We don't care about the order of the strings (or of !3 and !4). Let's just
+; make sure the strings are first and make it clear that there are two of them.
+; CHECK: <STRINGS {{.*}} num-strings = 2 {
+; CHECK-NEXT: 'leaf
+; CHECK-NEXT: 'leaf
+; CHECK-NEXT: }
+
+; The leafs should come first (in either order).
+; CHECK-NEXT: <NODE op0=1/>
+; CHECK-NEXT: <NODE op0=2/>
+!3 = !{!"leaf3"}
+!4 = !{!"leaf4"}
+
+; CHECK-NEXT: <NODE op0=3 op1=4/>
+!5 = !{!3, !4}
+
+; CHECK-NEXT: <NODE op0=3 op1=5 op2=4/>
+!6 = !{!3, !5, !4}
+
+; Note: named metadata nodes are not cannot reference null so their operands
+; are numbered off-by-one.
+; CHECK-NEXT: <NAME
+; CHECK-NEXT: <NAMED_NODE op0=5/>
+!named = !{!6}
Modified: llvm/trunk/test/Bitcode/metadata-function-blocks.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/metadata-function-blocks.ll?rev=266947&r1=266946&r2=266947&view=diff
==============================================================================
--- llvm/trunk/test/Bitcode/metadata-function-blocks.ll (original)
+++ llvm/trunk/test/Bitcode/metadata-function-blocks.ll Wed Apr 20 20:55:12 2016
@@ -19,14 +19,14 @@
; Each node gets a new number. Bottom-up traversal of nodes.
!named = !{!6}
-; CHECK-NEXT: <NODE op0=2/>
-!4 = !{!"named and foo"}
-
; CHECK-NEXT: <NODE op0=1/>
-!5 = !{!"named"}
+!4 = !{!"named"}
+
+; CHECK-NEXT: <NODE op0=2/>
+!5 = !{!"named and foo"}
-; CHECK-NEXT: <NODE op0=1 op1=5 op2=4/>
-!6 = !{!"named", !5, !4}
+; CHECK-NEXT: <NODE op0=1 op1=4 op2=5/>
+!6 = !{!"named", !4, !5}
; CHECK-NEXT: <NODE op0=3/>
!7 = !{!"foo and bar"}
More information about the llvm-commits
mailing list