diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h index 2fcb64206387e..d36f91416db88 100644 --- a/llvm/include/llvm/Transforms/Utils/Cloning.h +++ b/llvm/include/llvm/Transforms/Utils/Cloning.h @@ -193,7 +193,8 @@ void CloneFunctionAttributesInto(Function *NewFunc, const Function *OldFunc, void CloneFunctionMetadataInto(Function &NewFunc, const Function &OldFunc, ValueToValueMapTy &VMap, RemapFlags RemapFlag, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr); /// Clone OldFunc's body into NewFunc. void CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc, @@ -202,7 +203,8 @@ void CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc, const char *NameSuffix = "", ClonedCodeInfo *CodeInfo = nullptr, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr); void CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc, const Instruction *StartingInst, @@ -242,13 +244,24 @@ DISubprogram *CollectDebugInfoForCloning(const Function &F, CloneFunctionChangeType Changes, DebugInfoFinder &DIFinder); -/// Build a map of debug info to use during Metadata cloning. -/// Returns true if cloning would need module level changes and false if there -/// would only be local changes. -bool BuildDebugInfoMDMap(DenseMap &MD, - CloneFunctionChangeType Changes, - DebugInfoFinder &DIFinder, - DISubprogram *SPClonedWithinModule); +/// Based on \p Changes and \p DIFinder return debug info that needs to be +/// identity mapped during Metadata cloning. +/// +/// NOTE: Such \a MetadataSetTy can be used by \a CloneFunction* to directly +/// specify metadata that should be identity mapped (and hence not cloned). The +/// metadata will be identity mapped in \a ValueToValueMapTy on first use. There +/// are several reasons for doing it this way rather than eagerly identity +/// mapping metadata nodes in a \a ValueMap: +/// 1. Mapping metadata is not cheap, particularly because of tracking. +/// 2. When cloning a Function we identity map lots of global module-level +/// metadata to avoid cloning it, while only a fraction of it is actually +/// used by the function. Mapping on first use is a lot faster for modules +/// with meaningful amount of debug info. +/// 3. Eagerly identity mapping metadata makes it harder to cache module-level +/// data (e.g. a set of metadata nodes in a \a DICompileUnit). +MetadataSetTy FindDebugInfoToIdentityMap(CloneFunctionChangeType Changes, + DebugInfoFinder &DIFinder, + DISubprogram *SPClonedWithinModule); /// This class captures the data input to the InlineFunction call, and records /// the auxiliary results produced by it. diff --git a/llvm/include/llvm/Transforms/Utils/ValueMapper.h b/llvm/include/llvm/Transforms/Utils/ValueMapper.h index 743cfeb7ef3f0..852d7095d1133 100644 --- a/llvm/include/llvm/Transforms/Utils/ValueMapper.h +++ b/llvm/include/llvm/Transforms/Utils/ValueMapper.h @@ -15,6 +15,7 @@ #define LLVM_TRANSFORMS_UTILS_VALUEMAPPER_H #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/simple_ilist.h" #include "llvm/IR/ValueHandle.h" #include "llvm/IR/ValueMap.h" @@ -35,6 +36,7 @@ class Value; using ValueToValueMapTy = ValueMap; using DbgRecordIterator = simple_ilist::iterator; +using MetadataSetTy = SmallPtrSet; /// This is a class that can be implemented by clients to remap types when /// cloning constants and instructions. @@ -112,7 +114,7 @@ inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) { /// /// A shared context used for mapping and remapping of Value and Metadata /// instances using \a ValueToValueMapTy, \a RemapFlags, \a -/// ValueMapTypeRemapper, and \a ValueMaterializer. +/// ValueMapTypeRemapper, \a ValueMaterializer, and \a IdentityMD. /// /// There are a number of top-level entry points: /// - \a mapValue() (and \a mapConstant()); @@ -136,6 +138,9 @@ inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) { /// alternate \a ValueToValueMapTy and \a ValueMaterializer and returns a ID to /// pass into the schedule*() functions. /// +/// If an \a IdentityMD set is optionally provided, \a Metadata inside this set +/// will be mapped onto itself in \a VM on first use. +/// /// TODO: lib/Linker really doesn't need the \a ValueHandle in the \a /// ValueToValueMapTy. We should template \a ValueMapper (and its /// implementation classes), and explicitly instantiate on two concrete @@ -152,7 +157,8 @@ class ValueMapper { public: ValueMapper(ValueToValueMapTy &VM, RemapFlags Flags = RF_None, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr); ValueMapper(ValueMapper &&) = delete; ValueMapper(const ValueMapper &) = delete; ValueMapper &operator=(ValueMapper &&) = delete; @@ -218,8 +224,10 @@ class ValueMapper { inline Value *MapValue(const Value *V, ValueToValueMapTy &VM, RemapFlags Flags = RF_None, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr) { - return ValueMapper(VM, Flags, TypeMapper, Materializer).mapValue(*V); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr) { + return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD) + .mapValue(*V); } /// Lookup or compute a mapping for a piece of metadata. @@ -231,7 +239,9 @@ inline Value *MapValue(const Value *V, ValueToValueMapTy &VM, /// \c MD. /// 3. Else if \c MD is a \a ConstantAsMetadata, call \a MapValue() and /// re-wrap its return (returning nullptr on nullptr). -/// 4. Else, \c MD is an \a MDNode. These are remapped, along with their +/// 4. Else if \c MD is in \c IdentityMD then add an identity mapping for it +/// and return it. +/// 5. Else, \c MD is an \a MDNode. These are remapped, along with their /// transitive operands. Distinct nodes are duplicated or moved depending /// on \a RF_MoveDistinctNodes. Uniqued nodes are remapped like constants. /// @@ -240,16 +250,20 @@ inline Value *MapValue(const Value *V, ValueToValueMapTy &VM, inline Metadata *MapMetadata(const Metadata *MD, ValueToValueMapTy &VM, RemapFlags Flags = RF_None, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr) { - return ValueMapper(VM, Flags, TypeMapper, Materializer).mapMetadata(*MD); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr) { + return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD) + .mapMetadata(*MD); } /// Version of MapMetadata with type safety for MDNode. inline MDNode *MapMetadata(const MDNode *MD, ValueToValueMapTy &VM, RemapFlags Flags = RF_None, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr) { - return ValueMapper(VM, Flags, TypeMapper, Materializer).mapMDNode(*MD); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr) { + return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD) + .mapMDNode(*MD); } /// Convert the instruction operands from referencing the current values into @@ -263,8 +277,10 @@ inline MDNode *MapMetadata(const MDNode *MD, ValueToValueMapTy &VM, inline void RemapInstruction(Instruction *I, ValueToValueMapTy &VM, RemapFlags Flags = RF_None, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr) { - ValueMapper(VM, Flags, TypeMapper, Materializer).remapInstruction(*I); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr) { + ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD) + .remapInstruction(*I); } /// Remap the Values used in the DbgRecord \a DR using the value map \a @@ -272,8 +288,10 @@ inline void RemapInstruction(Instruction *I, ValueToValueMapTy &VM, inline void RemapDbgRecord(Module *M, DbgRecord *DR, ValueToValueMapTy &VM, RemapFlags Flags = RF_None, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr) { - ValueMapper(VM, Flags, TypeMapper, Materializer).remapDbgRecord(M, *DR); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr) { + ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD) + .remapDbgRecord(M, *DR); } /// Remap the Values used in the DbgRecords \a Range using the value map \a @@ -283,8 +301,9 @@ inline void RemapDbgRecordRange(Module *M, ValueToValueMapTy &VM, RemapFlags Flags = RF_None, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr) { - ValueMapper(VM, Flags, TypeMapper, Materializer) + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr) { + ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD) .remapDbgRecordRange(M, Range); } @@ -297,16 +316,19 @@ inline void RemapDbgRecordRange(Module *M, inline void RemapFunction(Function &F, ValueToValueMapTy &VM, RemapFlags Flags = RF_None, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr) { - ValueMapper(VM, Flags, TypeMapper, Materializer).remapFunction(F); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr) { + ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD).remapFunction(F); } /// Version of MapValue with type safety for Constant. inline Constant *MapValue(const Constant *V, ValueToValueMapTy &VM, RemapFlags Flags = RF_None, ValueMapTypeRemapper *TypeMapper = nullptr, - ValueMaterializer *Materializer = nullptr) { - return ValueMapper(VM, Flags, TypeMapper, Materializer).mapConstant(*V); + ValueMaterializer *Materializer = nullptr, + const MetadataSetTy *IdentityMD = nullptr) { + return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD) + .mapConstant(*V); } } // end namespace llvm diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp index 8863dff4482a1..127e7e6855a35 100644 --- a/llvm/lib/Transforms/Utils/CloneFunction.cpp +++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp @@ -152,61 +152,52 @@ DISubprogram *llvm::CollectDebugInfoForCloning(const Function &F, return SPClonedWithinModule; } -bool llvm::BuildDebugInfoMDMap(DenseMap &MD, - CloneFunctionChangeType Changes, - DebugInfoFinder &DIFinder, - DISubprogram *SPClonedWithinModule) { - bool ModuleLevelChanges = Changes > CloneFunctionChangeType::LocalChangesOnly; +MetadataSetTy +llvm::FindDebugInfoToIdentityMap(CloneFunctionChangeType Changes, + DebugInfoFinder &DIFinder, + DISubprogram *SPClonedWithinModule) { + MetadataSetTy MD; + if (Changes < CloneFunctionChangeType::DifferentModule && DIFinder.subprogram_count() > 0) { - // Turn on module-level changes, since we need to clone (some of) the - // debug info metadata. - // - // FIXME: Metadata effectively owned by a function should be made - // local, and only that local metadata should be cloned. - ModuleLevelChanges = true; - - auto mapToSelfIfNew = [&MD](MDNode *N) { - // Avoid clobbering an existing mapping. - (void)MD.try_emplace(N, N); - }; - // Avoid cloning types, compile units, and (other) subprograms. for (DISubprogram *ISP : DIFinder.subprograms()) { if (ISP != SPClonedWithinModule) - mapToSelfIfNew(ISP); + MD.insert(ISP); } // If a subprogram isn't going to be cloned skip its lexical blocks as well. for (DIScope *S : DIFinder.scopes()) { auto *LScope = dyn_cast(S); if (LScope && LScope->getSubprogram() != SPClonedWithinModule) - mapToSelfIfNew(S); + MD.insert(S); } for (DICompileUnit *CU : DIFinder.compile_units()) - mapToSelfIfNew(CU); + MD.insert(CU); for (DIType *Type : DIFinder.types()) - mapToSelfIfNew(Type); + MD.insert(Type); } else { assert(!SPClonedWithinModule && "Subprogram should be in DIFinder->subprogram_count()..."); } - return ModuleLevelChanges; + return MD; } void llvm::CloneFunctionMetadataInto(Function &NewFunc, const Function &OldFunc, ValueToValueMapTy &VMap, RemapFlags RemapFlag, ValueMapTypeRemapper *TypeMapper, - ValueMaterializer *Materializer) { + ValueMaterializer *Materializer, + const MetadataSetTy *IdentityMD) { SmallVector, 1> MDs; OldFunc.getAllMetadata(MDs); for (auto MD : MDs) { - NewFunc.addMetadata(MD.first, *MapMetadata(MD.second, VMap, RemapFlag, - TypeMapper, Materializer)); + NewFunc.addMetadata(MD.first, + *MapMetadata(MD.second, VMap, RemapFlag, TypeMapper, + Materializer, IdentityMD)); } } @@ -216,7 +207,8 @@ void llvm::CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc, const char *NameSuffix, ClonedCodeInfo *CodeInfo, ValueMapTypeRemapper *TypeMapper, - ValueMaterializer *Materializer) { + ValueMaterializer *Materializer, + const MetadataSetTy *IdentityMD) { if (OldFunc.isDeclaration()) return; @@ -258,9 +250,10 @@ void llvm::CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc, // Loop over all instructions, fixing each one as we find it, and any // attached debug-info records. for (Instruction &II : *BB) { - RemapInstruction(&II, VMap, RemapFlag, TypeMapper, Materializer); + RemapInstruction(&II, VMap, RemapFlag, TypeMapper, Materializer, + IdentityMD); RemapDbgRecordRange(II.getModule(), II.getDbgRecordRange(), VMap, - RemapFlag, TypeMapper, Materializer); + RemapFlag, TypeMapper, Materializer, IdentityMD); } } @@ -322,16 +315,19 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, DISubprogram *SPClonedWithinModule = CollectDebugInfoForCloning(*OldFunc, Changes, DIFinder); - ModuleLevelChanges = - BuildDebugInfoMDMap(VMap.MD(), Changes, DIFinder, SPClonedWithinModule); + MetadataSetTy IdentityMD = + FindDebugInfoToIdentityMap(Changes, DIFinder, SPClonedWithinModule); - const auto RemapFlag = ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges; + // Cloning is always a Module level operation, since Metadata needs to be + // cloned. + const auto RemapFlag = RF_None; CloneFunctionMetadataInto(*NewFunc, *OldFunc, VMap, RemapFlag, TypeMapper, - Materializer); + Materializer, &IdentityMD); CloneFunctionBodyInto(*NewFunc, *OldFunc, VMap, RemapFlag, Returns, - NameSuffix, CodeInfo, TypeMapper, Materializer); + NameSuffix, CodeInfo, TypeMapper, Materializer, + &IdentityMD); // Only update !llvm.dbg.cu for DifferentModule (not CloneModule). In the // same module, the compile unit will already be listed (or not). When diff --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp index 0b57c3bc538c6..b8569454379bf 100644 --- a/llvm/lib/Transforms/Utils/ValueMapper.cpp +++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp @@ -120,12 +120,14 @@ class Mapper { SmallVector Worklist; SmallVector DelayedBBs; SmallVector AppendingInits; + const MetadataSetTy *IdentityMD; public: Mapper(ValueToValueMapTy &VM, RemapFlags Flags, - ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer) + ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer, + const MetadataSetTy *IdentityMD) : Flags(Flags), TypeMapper(TypeMapper), - MCs(1, MappingContext(VM, Materializer)) {} + MCs(1, MappingContext(VM, Materializer)), IdentityMD(IdentityMD) {} /// ValueMapper should explicitly call \a flush() before destruction. ~Mapper() { assert(!hasWorkToDo() && "Expected to be flushed"); } @@ -899,6 +901,13 @@ std::optional Mapper::mapSimpleMetadata(const Metadata *MD) { return wrapConstantAsMetadata(*CMD, mapValue(CMD->getValue())); } + // Map metadata from IdentityMD on first use. We need to add these nodes to + // the mapping as otherwise metadata nodes numbering gets messed up. This is + // still economical because the amount of data in IdentityMD may be a lot + // larger than what will actually get used. + if (IdentityMD && IdentityMD->contains(MD)) + return getVM().MD()[MD] = TrackingMDRef(const_cast(MD)); + assert(isa(MD) && "Expected a metadata node"); return std::nullopt; @@ -1198,8 +1207,9 @@ class FlushingMapper { ValueMapper::ValueMapper(ValueToValueMapTy &VM, RemapFlags Flags, ValueMapTypeRemapper *TypeMapper, - ValueMaterializer *Materializer) - : pImpl(new Mapper(VM, Flags, TypeMapper, Materializer)) {} + ValueMaterializer *Materializer, + const MetadataSetTy *IdentityMD) + : pImpl(new Mapper(VM, Flags, TypeMapper, Materializer, IdentityMD)) {} ValueMapper::~ValueMapper() { delete getAsMapper(pImpl); }