diff --git a/llvm/include/llvm/Transforms/Utils/ValueMapper.h b/llvm/include/llvm/Transforms/Utils/ValueMapper.h index 047ab818711b..1419618110c8 100644 --- a/llvm/include/llvm/Transforms/Utils/ValueMapper.h +++ b/llvm/include/llvm/Transforms/Utils/ValueMapper.h @@ -59,7 +59,11 @@ namespace llvm { /// RF_IgnoreMissingEntries - If this flag is set, the remapper ignores /// entries that are not in the value map. If it is unset, it aborts if an /// operand is asked to be remapped which doesn't exist in the mapping. - RF_IgnoreMissingEntries = 2 + RF_IgnoreMissingEntries = 2, + + /// Instruct the remapper to move distinct metadata instead of duplicating + /// it when there are module-level changes. + RF_MoveDistinctMDs = 4, }; static inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) { diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp index f0906809ee48..e102119a1365 100644 --- a/llvm/lib/Linker/LinkModules.cpp +++ b/llvm/lib/Linker/LinkModules.cpp @@ -1156,7 +1156,7 @@ void ModuleLinker::linkAppendingVarInit(const AppendingVarInfo &AVI) { continue; } DstElements.push_back( - MapValue(V, ValueMap, RF_None, &TypeMap, &ValMaterializer)); + MapValue(V, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer)); } if (IsNewStructor) { NewType = ArrayType::get(NewType->getElementType(), DstElements.size()); @@ -1170,8 +1170,8 @@ void ModuleLinker::linkAppendingVarInit(const AppendingVarInfo &AVI) { /// referenced are in Dest. void ModuleLinker::linkGlobalInit(GlobalVariable &Dst, GlobalVariable &Src) { // Figure out what the initializer looks like in the dest module. - Dst.setInitializer(MapValue(Src.getInitializer(), ValueMap, RF_None, &TypeMap, - &ValMaterializer)); + Dst.setInitializer(MapValue(Src.getInitializer(), ValueMap, + RF_MoveDistinctMDs, &TypeMap, &ValMaterializer)); } /// Copy the source function over into the dest function and fix up references @@ -1186,18 +1186,20 @@ bool ModuleLinker::linkFunctionBody(Function &Dst, Function &Src) { // Link in the prefix data. if (Src.hasPrefixData()) - Dst.setPrefixData(MapValue(Src.getPrefixData(), ValueMap, RF_None, &TypeMap, - &ValMaterializer)); + Dst.setPrefixData(MapValue(Src.getPrefixData(), ValueMap, + RF_MoveDistinctMDs, &TypeMap, &ValMaterializer)); // Link in the prologue data. if (Src.hasPrologueData()) - Dst.setPrologueData(MapValue(Src.getPrologueData(), ValueMap, RF_None, - &TypeMap, &ValMaterializer)); + Dst.setPrologueData(MapValue(Src.getPrologueData(), ValueMap, + RF_MoveDistinctMDs, &TypeMap, + &ValMaterializer)); // Link in the personality function. if (Src.hasPersonalityFn()) - Dst.setPersonalityFn(MapValue(Src.getPersonalityFn(), ValueMap, RF_None, - &TypeMap, &ValMaterializer)); + Dst.setPersonalityFn(MapValue(Src.getPersonalityFn(), ValueMap, + RF_MoveDistinctMDs, &TypeMap, + &ValMaterializer)); // Go through and convert function arguments over, remembering the mapping. Function::arg_iterator DI = Dst.arg_begin(); @@ -1213,8 +1215,8 @@ bool ModuleLinker::linkFunctionBody(Function &Dst, Function &Src) { SmallVector, 8> MDs; Src.getAllMetadata(MDs); for (const auto &I : MDs) - Dst.setMetadata(I.first, MapMetadata(I.second, ValueMap, RF_None, &TypeMap, - &ValMaterializer)); + Dst.setMetadata(I.first, MapMetadata(I.second, ValueMap, RF_MoveDistinctMDs, + &TypeMap, &ValMaterializer)); // Splice the body of the source function into the dest function. Dst.getBasicBlockList().splice(Dst.end(), Src.getBasicBlockList()); @@ -1225,7 +1227,8 @@ bool ModuleLinker::linkFunctionBody(Function &Dst, Function &Src) { // functions and patch them up to point to the local versions. for (BasicBlock &BB : Dst) for (Instruction &I : BB) - RemapInstruction(&I, ValueMap, RF_IgnoreMissingEntries, &TypeMap, + RemapInstruction(&I, ValueMap, + RF_IgnoreMissingEntries | RF_MoveDistinctMDs, &TypeMap, &ValMaterializer); // There is no need to map the arguments anymore. @@ -1238,8 +1241,8 @@ bool ModuleLinker::linkFunctionBody(Function &Dst, Function &Src) { void ModuleLinker::linkAliasBody(GlobalAlias &Dst, GlobalAlias &Src) { Constant *Aliasee = Src.getAliasee(); - Constant *Val = - MapValue(Aliasee, ValueMap, RF_None, &TypeMap, &ValMaterializer); + Constant *Val = MapValue(Aliasee, ValueMap, RF_MoveDistinctMDs, &TypeMap, + &ValMaterializer); Dst.setAliasee(Val); } @@ -1266,8 +1269,8 @@ void ModuleLinker::linkNamedMDNodes() { NamedMDNode *DestNMD = DstM->getOrInsertNamedMetadata(NMD.getName()); // Add Src elements into Dest node. for (const MDNode *op : NMD.operands()) - DestNMD->addOperand( - MapMetadata(op, ValueMap, RF_None, &TypeMap, &ValMaterializer)); + DestNMD->addOperand(MapMetadata(op, ValueMap, RF_MoveDistinctMDs, + &TypeMap, &ValMaterializer)); } } @@ -1574,7 +1577,7 @@ bool ModuleLinker::run() { continue; const GlobalValue *GV = SrcM->getNamedValue(C.getName()); if (GV) - MapValue(GV, ValueMap, RF_None, &TypeMap, &ValMaterializer); + MapValue(GV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer); } // Strip replaced subprograms before mapping any metadata -- so that we're diff --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp index 8daf54688055..d967e0a95c6f 100644 --- a/llvm/lib/Transforms/Utils/ValueMapper.cpp +++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp @@ -216,9 +216,12 @@ static bool remap(const MDNode *OldNode, MDNode *NewNode, return AnyChanged; } -/// \brief Map a distinct MDNode. +/// Map a distinct MDNode. /// -/// Distinct nodes are not uniqued, so they must always recreated. +/// Whether distinct nodes change is independent of their operands. If \a +/// RF_MoveDistinctMDs, then they are reused, and their operands remapped in +/// place; effectively, they're moved from one graph to another. Otherwise, +/// they're cloned/duplicated, and the new copy's operands are remapped. static Metadata *mapDistinctNode(const MDNode *Node, SmallVectorImpl &Cycles, ValueToValueMapTy &VM, RemapFlags Flags, @@ -226,7 +229,11 @@ static Metadata *mapDistinctNode(const MDNode *Node, ValueMaterializer *Materializer) { assert(Node->isDistinct() && "Expected distinct node"); - MDNode *NewMD = MDNode::replaceWithDistinct(Node->clone()); + MDNode *NewMD; + if (Flags & RF_MoveDistinctMDs) + NewMD = const_cast(Node); + else + NewMD = MDNode::replaceWithDistinct(Node->clone()); // Remap the operands. If any change, track those that could be involved in // uniquing cycles. @@ -318,20 +325,21 @@ Metadata *llvm::MapMetadata(const Metadata *MD, ValueToValueMapTy &VM, Metadata *NewMD = MapMetadataImpl(MD, Cycles, VM, Flags, TypeMapper, Materializer); - // Resolve cycles underneath MD. - if (NewMD && NewMD != MD) { - if (auto *N = dyn_cast(NewMD)) - if (!N->isResolved()) - N->resolveCycles(); - - for (MDNode *N : Cycles) - if (!N->isResolved()) - N->resolveCycles(); - } else { - // Shouldn't get unresolved cycles if nothing was remapped. - assert(Cycles.empty() && "Expected no unresolved cycles"); + if ((Flags & RF_NoModuleLevelChanges) || + (MD == NewMD && !(Flags & RF_MoveDistinctMDs))) { + assert(Cycles.empty() && "Unresolved cycles without remapping anything?"); + return NewMD; } + if (auto *N = dyn_cast(NewMD)) + if (!N->isResolved()) + N->resolveCycles(); + + // Resolve cycles underneath MD. + for (MDNode *N : Cycles) + if (!N->isResolved()) + N->resolveCycles(); + return NewMD; } diff --git a/llvm/unittests/Linker/LinkModulesTest.cpp b/llvm/unittests/Linker/LinkModulesTest.cpp index 45f1308d3bd9..904ba58ce48e 100644 --- a/llvm/unittests/Linker/LinkModulesTest.cpp +++ b/llvm/unittests/Linker/LinkModulesTest.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "llvm/ADT/STLExtras.h" #include "llvm/AsmParser/Parser.h" #include "llvm/IR/BasicBlock.h" #include "llvm/IR/DataLayout.h" @@ -219,4 +220,77 @@ TEST_F(LinkModuleTest, CAPIFailure) { LLVMDisposeMessage(errout); } +TEST_F(LinkModuleTest, MoveDistinctMDs) { + LLVMContext C; + SMDiagnostic Err; + + const char *SrcStr = "define void @foo() !attach !0 {\n" + "entry:\n" + " call void @llvm.md(metadata !1)\n" + " ret void, !attach !2\n" + "}\n" + "declare void @llvm.md(metadata)\n" + "!named = !{!3, !4}\n" + "!0 = distinct !{}\n" + "!1 = distinct !{}\n" + "!2 = distinct !{}\n" + "!3 = distinct !{}\n" + "!4 = !{!3}\n"; + + std::unique_ptr Src = parseAssemblyString(SrcStr, Err, C); + assert(Src); + ASSERT_TRUE(Src.get()); + + // Get the addresses of the Metadata before merging. + Function *F = &*Src->begin(); + ASSERT_EQ("foo", F->getName()); + BasicBlock *BB = &F->getEntryBlock(); + auto *CI = cast(&BB->front()); + auto *RI = cast(BB->getTerminator()); + NamedMDNode *NMD = &*Src->named_metadata_begin(); + + MDNode *M0 = F->getMetadata("attach"); + MDNode *M1 = + cast(cast(CI->getArgOperand(0))->getMetadata()); + MDNode *M2 = RI->getMetadata("attach"); + MDNode *M3 = NMD->getOperand(0); + MDNode *M4 = NMD->getOperand(1); + + // Confirm a few things about the IR. + EXPECT_TRUE(M0->isDistinct()); + EXPECT_TRUE(M1->isDistinct()); + EXPECT_TRUE(M2->isDistinct()); + EXPECT_TRUE(M3->isDistinct()); + EXPECT_TRUE(M4->isUniqued()); + EXPECT_EQ(M3, M4->getOperand(0)); + + // Link into destination module. + auto Dst = llvm::make_unique("Linked", C); + ASSERT_TRUE(Dst.get()); + Linker::LinkModules(Dst.get(), Src.get(), + [](const llvm::DiagnosticInfo &) {}); + + // Check that distinct metadata was moved, not cloned. Even !4, the uniqued + // node, should effectively be moved, since its only operand hasn't changed. + F = &*Dst->begin(); + BB = &F->getEntryBlock(); + CI = cast(&BB->front()); + RI = cast(BB->getTerminator()); + NMD = &*Dst->named_metadata_begin(); + + EXPECT_EQ(M0, F->getMetadata("attach")); + EXPECT_EQ(M1, cast(CI->getArgOperand(0))->getMetadata()); + EXPECT_EQ(M2, RI->getMetadata("attach")); + EXPECT_EQ(M3, NMD->getOperand(0)); + EXPECT_EQ(M4, NMD->getOperand(1)); + + // Confirm a few things about the IR. This shouldn't have changed. + EXPECT_TRUE(M0->isDistinct()); + EXPECT_TRUE(M1->isDistinct()); + EXPECT_TRUE(M2->isDistinct()); + EXPECT_TRUE(M3->isDistinct()); + EXPECT_TRUE(M4->isUniqued()); + EXPECT_EQ(M3, M4->getOperand(0)); +} + } // end anonymous namespace diff --git a/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp b/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp index 137a2607c848..9dbe4dbc56de 100644 --- a/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp +++ b/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp @@ -24,4 +24,35 @@ TEST(ValueMapperTest, MapMetadataUnresolved) { EXPECT_EQ(T.get(), MapMetadata(T.get(), VM, RF_NoModuleLevelChanges)); } +TEST(ValueMapperTest, MapMetadataDistinct) { + LLVMContext Context; + auto *D = MDTuple::getDistinct(Context, None); + + { + // The node should be cloned. + ValueToValueMapTy VM; + EXPECT_NE(D, MapMetadata(D, VM, RF_None)); + } + { + // The node should be moved. + ValueToValueMapTy VM; + EXPECT_EQ(D, MapMetadata(D, VM, RF_MoveDistinctMDs)); + } +} + +TEST(ValueMapperTest, MapMetadataDistinctOperands) { + LLVMContext Context; + Metadata *Old = MDTuple::getDistinct(Context, None); + auto *D = MDTuple::getDistinct(Context, Old); + ASSERT_EQ(Old, D->getOperand(0)); + + Metadata *New = MDTuple::getDistinct(Context, None); + ValueToValueMapTy VM; + VM.MD()[Old].reset(New); + + // Make sure operands are updated. + EXPECT_EQ(D, MapMetadata(D, VM, RF_MoveDistinctMDs)); + EXPECT_EQ(New, D->getOperand(0)); +} + }