From 4fb46cb818f43805205cf05d7064cb443e04f69c Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Mon, 3 Aug 2015 17:09:38 +0000 Subject: Linker: Move distinct MDNodes instead of cloning Instead of cloning distinct `MDNode`s when linking in a module, just move them over. The module linker destroys the source module, so the old node would otherwise just be leaked on the context. Create the new node in place. This also reduces the number of cloned uniqued nodes (since it's less likely their operands have changed). This mapping strategy is only correct when we're discarding the source, so the linker turns it on via a ValueMapper flag, `RF_MoveDistinctMDs`. There's nothing observable in terms of `llvm-link` output here: the linked module should be semantically identical. I'll be adding more 'distinct' nodes to the debug info metadata graph in order to break uniquing cycles, so the benefits of this will partly come in future commits. However, we should get some gains immediately, since we have a fair number of 'distinct' `DILocation`s being linked in. llvm-svn: 243883 --- llvm/lib/Linker/LinkModules.cpp | 37 ++++++++++++++++-------------- llvm/lib/Transforms/Utils/ValueMapper.cpp | 38 +++++++++++++++++++------------ 2 files changed, 43 insertions(+), 32 deletions(-) (limited to 'llvm/lib') diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp index f0906809ee4..e102119a136 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 8daf5468805..d967e0a95c6 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; } -- cgit v1.2.3