From a415938d70c829915296ed4d3328636ed2913f49 Mon Sep 17 00:00:00 2001 From: Artem Pianykh Date: Wed, 20 Nov 2024 05:19:19 -0700 Subject: [PATCH 1/5] [Utils] Extract ProcessSubprogramAttachment from CloneFunctionInto Summary: Consolidate the logic in a single function. We do an extra pass over Instructions but this is necessary to untangle things and extract metadata cloning in a future diff. Test Plan: ninja check-llvm-unit --- llvm/include/llvm/Transforms/Utils/Cloning.h | 9 +++ llvm/lib/Transforms/Utils/CloneFunction.cpp | 61 +++++++++++++------- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h index 1e8ef0102450e..c5be1cce4fff8 100644 --- a/llvm/include/llvm/Transforms/Utils/Cloning.h +++ b/llvm/include/llvm/Transforms/Utils/Cloning.h @@ -207,6 +207,15 @@ void CloneAndPruneFunctionInto(Function *NewFunc, const Function *OldFunc, const char *NameSuffix = "", ClonedCodeInfo *CodeInfo = nullptr); +/// Process function's subprogram attachment to collect relevant debug +/// information in DIFinder. +/// +/// Returns DISubprogram of the cloned function when cloning into the same +/// module or nullptr otherwise. +DISubprogram *ProcessSubprogramAttachment(const Function &F, + CloneFunctionChangeType Changes, + DebugInfoFinder &DIFinder); + /// This class captures the data input to the InlineFunction call, and records /// the auxiliary results produced by it. class InlineFunctionInfo { diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp index 4c8a5558b348c..bacf27895efc9 100644 --- a/llvm/lib/Transforms/Utils/CloneFunction.cpp +++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp @@ -136,6 +136,29 @@ void llvm::CloneFunctionAttributesInto(Function *NewFunc, OldAttrs.getRetAttrs(), NewArgAttrs)); } +DISubprogram *llvm::ProcessSubprogramAttachment(const Function &F, + CloneFunctionChangeType Changes, + DebugInfoFinder &DIFinder) { + DISubprogram *SPClonedWithinModule = nullptr; + if (Changes < CloneFunctionChangeType::DifferentModule) { + SPClonedWithinModule = F.getSubprogram(); + } + if (SPClonedWithinModule) + DIFinder.processSubprogram(SPClonedWithinModule); + + const Module *M = F.getParent(); + if (Changes != CloneFunctionChangeType::ClonedModule && M) { + // Inspect instructions to process e.g. DILexicalBlocks of inlined functions + for (const auto &BB : F) { + for (const auto &I : BB) { + DIFinder.processInstruction(*M, I); + } + } + } + + return SPClonedWithinModule; +} + // Clone OldFunc into NewFunc, transforming the old arguments into references to // VMap values. void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, @@ -168,23 +191,19 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, // duplicate instructions and then freeze them in the MD map. We also record // information about dbg.value and dbg.declare to avoid duplicating the // types. - std::optional DIFinder; + DebugInfoFinder DIFinder; // Track the subprogram attachment that needs to be cloned to fine-tune the // mapping within the same module. - DISubprogram *SPClonedWithinModule = nullptr; if (Changes < CloneFunctionChangeType::DifferentModule) { + // Need to find subprograms, types, and compile units. + assert((NewFunc->getParent() == nullptr || NewFunc->getParent() == OldFunc->getParent()) && "Expected NewFunc to have the same parent, or no parent"); - - // Need to find subprograms, types, and compile units. - DIFinder.emplace(); - - SPClonedWithinModule = OldFunc->getSubprogram(); - if (SPClonedWithinModule) - DIFinder->processSubprogram(SPClonedWithinModule); } else { + // Need to find all the compile units. + assert((NewFunc->getParent() == nullptr || NewFunc->getParent() != OldFunc->getParent()) && "Expected NewFunc to have different parents, or no parent"); @@ -192,20 +211,22 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, if (Changes == CloneFunctionChangeType::DifferentModule) { assert(NewFunc->getParent() && "Need parent of new function to maintain debug info invariants"); - - // Need to find all the compile units. - DIFinder.emplace(); } } + DISubprogram *SPClonedWithinModule = + ProcessSubprogramAttachment(*OldFunc, Changes, DIFinder); + // Loop over all of the basic blocks in the function, cloning them as // appropriate. Note that we save BE this way in order to handle cloning of // recursive functions into themselves. for (const BasicBlock &BB : *OldFunc) { // Create a new basic block and copy instructions into it! - BasicBlock *CBB = CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo, - DIFinder ? &*DIFinder : nullptr); + // NOTE: don't pass DIFinder because instructions' debug info was processed + // in ProcessSubprogramAttachment. This will be cleaned up further. + BasicBlock *CBB = + CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo, nullptr); // Add basic block mapping. VMap[&BB] = CBB; @@ -228,7 +249,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, } if (Changes < CloneFunctionChangeType::DifferentModule && - DIFinder->subprogram_count() > 0) { + DIFinder.subprogram_count() > 0) { // Turn on module-level changes, since we need to clone (some of) the // debug info metadata. // @@ -243,7 +264,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, // Avoid cloning types, compile units, and (other) subprograms. SmallPtrSet MappedToSelfSPs; - for (DISubprogram *ISP : DIFinder->subprograms()) { + for (DISubprogram *ISP : DIFinder.subprograms()) { if (ISP != SPClonedWithinModule) { mapToSelfIfNew(ISP); MappedToSelfSPs.insert(ISP); @@ -251,16 +272,16 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, } // If a subprogram isn't going to be cloned skip its lexical blocks as well. - for (DIScope *S : DIFinder->scopes()) { + for (DIScope *S : DIFinder.scopes()) { auto *LScope = dyn_cast(S); if (LScope && MappedToSelfSPs.count(LScope->getSubprogram())) mapToSelfIfNew(S); } - for (DICompileUnit *CU : DIFinder->compile_units()) + for (DICompileUnit *CU : DIFinder.compile_units()) mapToSelfIfNew(CU); - for (DIType *Type : DIFinder->types()) + for (DIType *Type : DIFinder.types()) mapToSelfIfNew(Type); } else { assert(!SPClonedWithinModule && @@ -314,7 +335,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, SmallPtrSet Visited; for (auto *Operand : NMD->operands()) Visited.insert(Operand); - for (auto *Unit : DIFinder->compile_units()) { + for (auto *Unit : DIFinder.compile_units()) { MDNode *MappedUnit = MapMetadata(Unit, VMap, RF_None, TypeMapper, Materializer); if (Visited.insert(MappedUnit).second) From b600fb42efacccc7cddf875bd79cdcc0669953c3 Mon Sep 17 00:00:00 2001 From: Artem Pianykh Date: Tue, 12 Nov 2024 02:04:36 -0800 Subject: [PATCH 2/5] fixup! [Utils] Extract ProcessSubprogramAttachment from CloneFunctionInto --- llvm/lib/Transforms/Utils/CloneFunction.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp index bacf27895efc9..3549c3acdcd37 100644 --- a/llvm/lib/Transforms/Utils/CloneFunction.cpp +++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp @@ -23,6 +23,7 @@ #include "llvm/IR/DebugInfo.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/Function.h" +#include "llvm/IR/InstIterator.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/LLVMContext.h" @@ -149,11 +150,8 @@ DISubprogram *llvm::ProcessSubprogramAttachment(const Function &F, const Module *M = F.getParent(); if (Changes != CloneFunctionChangeType::ClonedModule && M) { // Inspect instructions to process e.g. DILexicalBlocks of inlined functions - for (const auto &BB : F) { - for (const auto &I : BB) { - DIFinder.processInstruction(*M, I); - } - } + for (const auto &I : instructions(F)) + DIFinder.processInstruction(*M, I); } return SPClonedWithinModule; From e70ed597a4204b9a595efbc8f01c7d54983fee98 Mon Sep 17 00:00:00 2001 From: Artem Pianykh Date: Tue, 12 Nov 2024 02:42:46 -0800 Subject: [PATCH 3/5] amend! [Utils] Extract CollectDebugInfoForCloning from CloneFunctionInto --- llvm/include/llvm/Transforms/Utils/Cloning.h | 15 ++++++++++----- llvm/lib/Transforms/Utils/CloneFunction.cpp | 8 ++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h index c5be1cce4fff8..28a2dd79e10a2 100644 --- a/llvm/include/llvm/Transforms/Utils/Cloning.h +++ b/llvm/include/llvm/Transforms/Utils/Cloning.h @@ -207,14 +207,19 @@ void CloneAndPruneFunctionInto(Function *NewFunc, const Function *OldFunc, const char *NameSuffix = "", ClonedCodeInfo *CodeInfo = nullptr); -/// Process function's subprogram attachment to collect relevant debug -/// information in DIFinder. +/// Collect debug information such as types, compile units, and other +/// subprograms that are reachable from \p F and can be considered global for +/// the purposes of cloning (and hence not needing to be cloned). +/// +/// The latter depends on \p Changes: when cloning into the same module we +/// process \p F's subprogram and instructions; when into a cloned module, +/// neither of those. /// /// Returns DISubprogram of the cloned function when cloning into the same /// module or nullptr otherwise. -DISubprogram *ProcessSubprogramAttachment(const Function &F, - CloneFunctionChangeType Changes, - DebugInfoFinder &DIFinder); +DISubprogram *CollectDebugInfoForCloning(const Function &F, + CloneFunctionChangeType Changes, + DebugInfoFinder &DIFinder); /// This class captures the data input to the InlineFunction call, and records /// the auxiliary results produced by it. diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp index 3549c3acdcd37..cb6a4e34c226e 100644 --- a/llvm/lib/Transforms/Utils/CloneFunction.cpp +++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp @@ -137,9 +137,9 @@ void llvm::CloneFunctionAttributesInto(Function *NewFunc, OldAttrs.getRetAttrs(), NewArgAttrs)); } -DISubprogram *llvm::ProcessSubprogramAttachment(const Function &F, - CloneFunctionChangeType Changes, - DebugInfoFinder &DIFinder) { +DISubprogram *llvm::CollectDebugInfoForCloning(const Function &F, + CloneFunctionChangeType Changes, + DebugInfoFinder &DIFinder) { DISubprogram *SPClonedWithinModule = nullptr; if (Changes < CloneFunctionChangeType::DifferentModule) { SPClonedWithinModule = F.getSubprogram(); @@ -213,7 +213,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, } DISubprogram *SPClonedWithinModule = - ProcessSubprogramAttachment(*OldFunc, Changes, DIFinder); + CollectDebugInfoForCloning(*OldFunc, Changes, DIFinder); // Loop over all of the basic blocks in the function, cloning them as // appropriate. Note that we save BE this way in order to handle cloning of From 8723505598d14fe1d3b20fe3c23771827ef11080 Mon Sep 17 00:00:00 2001 From: Artem Pyanykh Date: Wed, 20 Nov 2024 19:16:51 +0000 Subject: [PATCH 4/5] fixup! amend! [Utils] Extract CollectDebugInfoForCloning from CloneFunctionInto --- llvm/include/llvm/Transforms/Utils/Cloning.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h index 28a2dd79e10a2..d254c3f616c22 100644 --- a/llvm/include/llvm/Transforms/Utils/Cloning.h +++ b/llvm/include/llvm/Transforms/Utils/Cloning.h @@ -211,9 +211,9 @@ void CloneAndPruneFunctionInto(Function *NewFunc, const Function *OldFunc, /// subprograms that are reachable from \p F and can be considered global for /// the purposes of cloning (and hence not needing to be cloned). /// -/// The latter depends on \p Changes: when cloning into the same module we -/// process \p F's subprogram and instructions; when into a cloned module, -/// neither of those. +/// What debug information is considered global depends on \p Changes: when +/// cloning into the same module we process \p F's subprogram and instructions; +/// when into a cloned module, neither of those. /// /// Returns DISubprogram of the cloned function when cloning into the same /// module or nullptr otherwise. From dc56df80bbd41aa7ae4020304e56dbeb66c42233 Mon Sep 17 00:00:00 2001 From: Artem Pyanykh Date: Wed, 20 Nov 2024 19:19:27 +0000 Subject: [PATCH 5/5] fixup! fixup! amend! [Utils] Extract CollectDebugInfoForCloning from CloneFunctionInto --- llvm/include/llvm/Transforms/Utils/Cloning.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h index d254c3f616c22..049d68b8a3068 100644 --- a/llvm/include/llvm/Transforms/Utils/Cloning.h +++ b/llvm/include/llvm/Transforms/Utils/Cloning.h @@ -211,7 +211,7 @@ void CloneAndPruneFunctionInto(Function *NewFunc, const Function *OldFunc, /// subprograms that are reachable from \p F and can be considered global for /// the purposes of cloning (and hence not needing to be cloned). /// -/// What debug information is considered global depends on \p Changes: when +/// What debug information should be processed depends on \p Changes: when /// cloning into the same module we process \p F's subprogram and instructions; /// when into a cloned module, neither of those. ///