[Utils] Extract CollectDebugInfoForCloning from CloneFunctionInto

[artempyanykh]

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 check-llvm
[211/213] Running the LLVM regression tests

Testing Time: 106.06s

Total Discovered Tests: 62601
  Skipped          :    17 (0.03%)
  Unsupported      :  2518 (4.02%)
  Passed           : 59911 (95.70%)
  Expectedly Failed:   155 (0.25%)
[212/213] Running lit suite 

Testing Time: 12.47s

Total Discovered Tests: 8474
  Skipped:   17 (0.20%)
  Passed : 8457 (99.80%)

Extracted from #109032 (commit 3) (there are more refactors and cleanups in subsequent commits)

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Artem Pianykh (artempyanykh)

Changes

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

Extracted from #109032 (commit 3) (there are more refactors and cleanups in subsequent commits)

---

Full diff: https://github.com/llvm/llvm-project/pull/114537.diff

2 Files Affected:

• (modified) llvm/include/llvm/Transforms/Utils/Cloning.h (+9)
• (modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+41-19)

diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h
index 1e8ef0102450e4..c5be1cce4fff89 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 a2d38717f38d14..3b971d972e11ed 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -137,6 +137,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,
@@ -169,23 +192,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<DebugInfoFinder> 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");
@@ -194,19 +213,22 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
       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 becase instructions' debug info was process in
+    // ProcessSubprogramAttachment. This will be further cleaned up.
+    BasicBlock *CBB =
+        CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo, nullptr);
 
     // Add basic block mapping.
     VMap[&BB] = CBB;
@@ -229,7 +251,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.
     //
@@ -244,7 +266,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
 
     // Avoid cloning types, compile units, and (other) subprograms.
     SmallPtrSet<const DISubprogram *, 16> MappedToSelfSPs;
-    for (DISubprogram *ISP : DIFinder->subprograms()) {
+    for (DISubprogram *ISP : DIFinder.subprograms()) {
       if (ISP != SPClonedWithinModule) {
         mapToSelfIfNew(ISP);
         MappedToSelfSPs.insert(ISP);
@@ -252,16 +274,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<DILocalScope>(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 &&
@@ -315,7 +337,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
   SmallPtrSet<const void *, 8> 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)

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[felipepiovezan]

Test Plan:
ninja check-llvm-unit

Could you run check-llvm? I don't think the unit tests will run the LIT tests as well, which impact the coro-splitter

[artempyanykh]

Could you run check-llvm? I don't think the unit tests will run the LIT tests as well, which impact the coro-splitter

Sure! Did that and updated the test plan.

[artempyanykh]

Rebased on trunk

[felipepiovezan]

LGTM but left one comment about documentation!