[llvm] r225397 - Linker: Don't use MDNode::replaceOperandWith()
Duncan P. N. Exon Smith
dexonsmith at apple.com
Wed Jan 7 13:32:28 PST 2015
Author: dexonsmith
Date: Wed Jan 7 15:32:27 2015
New Revision: 225397
URL: http://llvm.org/viewvc/llvm-project?rev=225397&view=rev
Log:
Linker: Don't use MDNode::replaceOperandWith()
`MDNode::replaceOperandWith()` changes all instances of metadata. Stop
using it when linking module flags, since (due to uniquing) the flag
values could be used by other metadata.
Instead, use new API `NamedMDNode::setOperand()` to update the reference
directly.
Added:
llvm/trunk/test/Linker/Inputs/module-flags-dont-change-others.ll
llvm/trunk/test/Linker/module-flags-dont-change-others.ll
Modified:
llvm/trunk/include/llvm/IR/Metadata.h
llvm/trunk/lib/IR/Metadata.cpp
llvm/trunk/lib/Linker/LinkModules.cpp
Modified: llvm/trunk/include/llvm/IR/Metadata.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=225397&r1=225396&r2=225397&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Metadata.h (original)
+++ llvm/trunk/include/llvm/IR/Metadata.h Wed Jan 7 15:32:27 2015
@@ -847,6 +847,7 @@ public:
MDNode *getOperand(unsigned i) const;
unsigned getNumOperands() const;
void addOperand(MDNode *M);
+ void setOperand(unsigned I, MDNode *New);
StringRef getName() const;
void print(raw_ostream &ROS) const;
void dump() const;
Modified: llvm/trunk/lib/IR/Metadata.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=225397&r1=225396&r2=225397&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Metadata.cpp (original)
+++ llvm/trunk/lib/IR/Metadata.cpp Wed Jan 7 15:32:27 2015
@@ -836,6 +836,11 @@ MDNode *NamedMDNode::getOperand(unsigned
void NamedMDNode::addOperand(MDNode *M) { getNMDOps(Operands).emplace_back(M); }
+void NamedMDNode::setOperand(unsigned I, MDNode *New) {
+ assert(I < getNumOperands() && "Invalid operand number");
+ getNMDOps(Operands)[I].reset(New);
+}
+
void NamedMDNode::eraseFromParent() {
getParent()->eraseNamedMetadata(this);
}
Modified: llvm/trunk/lib/Linker/LinkModules.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=225397&r1=225396&r2=225397&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/LinkModules.cpp (original)
+++ llvm/trunk/lib/Linker/LinkModules.cpp Wed Jan 7 15:32:27 2015
@@ -1312,7 +1312,7 @@ bool ModuleLinker::linkModuleFlagsMetada
}
// First build a map of the existing module flags and requirements.
- DenseMap<MDString*, MDNode*> Flags;
+ DenseMap<MDString *, std::pair<MDNode *, unsigned>> Flags;
SmallSetVector<MDNode*, 16> Requirements;
for (unsigned I = 0, E = DstModFlags->getNumOperands(); I != E; ++I) {
MDNode *Op = DstModFlags->getOperand(I);
@@ -1322,7 +1322,7 @@ bool ModuleLinker::linkModuleFlagsMetada
if (Behavior->getZExtValue() == Module::Require) {
Requirements.insert(cast<MDNode>(Op->getOperand(2)));
} else {
- Flags[ID] = Op;
+ Flags[ID] = std::make_pair(Op, I);
}
}
@@ -1334,7 +1334,9 @@ bool ModuleLinker::linkModuleFlagsMetada
ConstantInt *SrcBehavior =
mdconst::extract<ConstantInt>(SrcOp->getOperand(0));
MDString *ID = cast<MDString>(SrcOp->getOperand(1));
- MDNode *DstOp = Flags.lookup(ID);
+ MDNode *DstOp;
+ unsigned DstIndex;
+ std::tie(DstOp, DstIndex) = Flags.lookup(ID);
unsigned SrcBehaviorValue = SrcBehavior->getZExtValue();
// If this is a requirement, add it and continue.
@@ -1349,7 +1351,7 @@ bool ModuleLinker::linkModuleFlagsMetada
// If there is no existing flag with this ID, just add it.
if (!DstOp) {
- Flags[ID] = SrcOp;
+ Flags[ID] = std::make_pair(SrcOp, DstModFlags->getNumOperands());
DstModFlags->addOperand(SrcOp);
continue;
}
@@ -1370,8 +1372,8 @@ bool ModuleLinker::linkModuleFlagsMetada
continue;
} else if (SrcBehaviorValue == Module::Override) {
// Update the destination flag to that of the source.
- DstOp->replaceOperandWith(0, ConstantAsMetadata::get(SrcBehavior));
- DstOp->replaceOperandWith(2, SrcOp->getOperand(2));
+ DstModFlags->setOperand(DstIndex, SrcOp);
+ Flags[ID].first = SrcOp;
continue;
}
@@ -1382,6 +1384,13 @@ bool ModuleLinker::linkModuleFlagsMetada
continue;
}
+ auto replaceDstValue = [&](MDNode *New) {
+ Metadata *FlagOps[] = {DstOp->getOperand(0), ID, New};
+ MDNode *Flag = MDNode::get(DstM->getContext(), FlagOps);
+ DstModFlags->setOperand(DstIndex, Flag);
+ Flags[ID].first = Flag;
+ };
+
// Perform the merge for standard behavior types.
switch (SrcBehaviorValue) {
case Module::Require:
@@ -1411,7 +1420,8 @@ bool ModuleLinker::linkModuleFlagsMetada
MDs.push_back(DstValue->getOperand(i));
for (unsigned i = 0, e = SrcValue->getNumOperands(); i != e; ++i)
MDs.push_back(SrcValue->getOperand(i));
- DstOp->replaceOperandWith(2, MDNode::get(DstM->getContext(), MDs));
+
+ replaceDstValue(MDNode::get(DstM->getContext(), MDs));
break;
}
case Module::AppendUnique: {
@@ -1422,9 +1432,9 @@ bool ModuleLinker::linkModuleFlagsMetada
Elts.insert(DstValue->getOperand(i));
for (unsigned i = 0, e = SrcValue->getNumOperands(); i != e; ++i)
Elts.insert(SrcValue->getOperand(i));
- DstOp->replaceOperandWith(
- 2, MDNode::get(DstM->getContext(),
- makeArrayRef(Elts.begin(), Elts.end())));
+
+ replaceDstValue(MDNode::get(DstM->getContext(),
+ makeArrayRef(Elts.begin(), Elts.end())));
break;
}
}
@@ -1436,7 +1446,7 @@ bool ModuleLinker::linkModuleFlagsMetada
MDString *Flag = cast<MDString>(Requirement->getOperand(0));
Metadata *ReqValue = Requirement->getOperand(1);
- MDNode *Op = Flags[Flag];
+ MDNode *Op = Flags[Flag].first;
if (!Op || Op->getOperand(2) != ReqValue) {
HasErr |= emitError("linking module flags '" + Flag->getString() +
"': does not have the required value");
Added: llvm/trunk/test/Linker/Inputs/module-flags-dont-change-others.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/module-flags-dont-change-others.ll?rev=225397&view=auto
==============================================================================
--- llvm/trunk/test/Linker/Inputs/module-flags-dont-change-others.ll (added)
+++ llvm/trunk/test/Linker/Inputs/module-flags-dont-change-others.ll Wed Jan 7 15:32:27 2015
@@ -0,0 +1,8 @@
+!llvm.module.flags = !{!3, !4, !5}
+
+!0 = !{}
+!1 = !{!0}
+!2 = !{!0, !1}
+!3 = !{i32 4, !"foo", i32 37} ; Override the "foo" value.
+!4 = !{i32 5, !"bar", !1}
+!5 = !{i32 6, !"baz", !2}
Added: llvm/trunk/test/Linker/module-flags-dont-change-others.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/module-flags-dont-change-others.ll?rev=225397&view=auto
==============================================================================
--- llvm/trunk/test/Linker/module-flags-dont-change-others.ll (added)
+++ llvm/trunk/test/Linker/module-flags-dont-change-others.ll Wed Jan 7 15:32:27 2015
@@ -0,0 +1,26 @@
+; RUN: llvm-link %s %S/Inputs/module-flags-dont-change-others.ll -S -o - | FileCheck %s
+
+; Test that module-flag linking doesn't change other metadata. In particular,
+; !named should still point at the unmodified tuples (!3, !4, and !5) that
+; happen to also serve as module flags.
+
+; CHECK: !named = !{!0, !1, !2, !3, !4, !5}
+; CHECK: !llvm.module.flags = !{!6, !7, !8}
+!named = !{!0, !1, !2, !3, !4, !5}
+!llvm.module.flags = !{!3, !4, !5}
+
+; CHECK: !0 = !{}
+; CHECK: !1 = !{!0}
+; CHECK: !2 = !{!0, !1}
+; CHECK: !3 = !{i32 1, !"foo", i32 927}
+; CHECK: !4 = !{i32 5, !"bar", !0}
+; CHECK: !5 = !{i32 6, !"baz", !1}
+; CHECK: !6 = !{i32 4, !"foo", i32 37}
+; CHECK: !7 = !{i32 5, !"bar", !1}
+; CHECK: !8 = !{i32 6, !"baz", !2}
+!0 = !{}
+!1 = !{!0}
+!2 = !{!0, !1}
+!3 = !{i32 1, !"foo", i32 927}
+!4 = !{i32 5, !"bar", !0}
+!5 = !{i32 6, !"baz", !1}
More information about the llvm-commits
mailing list