[NFC][Cloning] Make DifferentModule case more obvious in CollectDebugInfoForCloning

[artempyanykh]

Stacked PRs:

• [NFC][Cloning] Move DebugInfoFinder decl closer to its place of usage #129154
• [NFC][Cloning] Clean up comments in CloneFunctionInto #129153
• [NFC][Cloning] Remove now unused CollectDebugInfoForCloning #129152
• [NFC][Cloning] Remove now unused FindDebugInfoToIdentityMap #129151
• [NFC][Coro] Remove now unused CommonDebugInfo in CoroSplit #129150
• [NFC][Coro] Use CloneFunctionInto for coroutine cloning instead of CloneFunction<Part> #129149
• [NFC][Cloning] Replace DIFinder usage in CloneFunctionInto with a MetadataPredicate #129148
• [NFC][Cloning] Replace IdentityMD set with a predicate in ValueMapper #129147
• ->[NFC][Cloning] Make DifferentModule case more obvious in CollectDebugInfoForCloning #129146
• [NFC][Cloning] Add a helper to collect debug info from instructions #129145
• [NFC][Cloning] Simplify the flow in FindDebugInfoToIdentityMap #129144
• [NFC][Cloning] Make ClonedModule case more obvious in CollectDebugInfoForCloning #129143

---

[NFC][Cloning] Make DifferentModule case more obvious in CollectDebugInfoForCloning

Summary:
This should be behaviorally equivalent. DIFinder is only used when
cloning into a DifferentModule as part of llvm.dbg.cu update in
CloneFunctionInto.

Test Plan:
ninja check-llvm-unit check-llvm

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Artem Pianykh (artempyanykh)

Changes

[NFC][Cloning] Make DifferentModule case more obvious in CollectDebugInfoForCloning

Summary:
This should be behaviorally equivalent. DIFinder is only used when
cloning into a DifferentModule as part of llvm.dbg.cu update in
CloneFunctionInto.

Test Plan:
ninja check-llvm-unit check-llvm

---

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

1 Files Affected:

• (modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+8-2)

diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 9ea4e64671c7e..6b6315d698c9a 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -147,8 +147,10 @@ void llvm::CloneFunctionAttributesInto(Function *NewFunc,
 DISubprogram *llvm::CollectDebugInfoForCloning(const Function &F,
                                                CloneFunctionChangeType Changes,
                                                DebugInfoFinder &DIFinder) {
-  // CloneModule takes care of cloning debug info.
-  if (Changes == CloneFunctionChangeType::ClonedModule)
+  // CloneModule takes care of cloning debug info for ClonedModule. Cloning into
+  // DifferentModule is taken care of separately in ClonedFunctionInto as part
+  // of llvm.dbg.cu update.
+  if (Changes >= CloneFunctionChangeType::DifferentModule)
     return nullptr;
 
   DISubprogram *SPClonedWithinModule = nullptr;
@@ -362,6 +364,10 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
   SmallPtrSet<const void *, 8> Visited;
   for (auto *Operand : NMD->operands())
     Visited.insert(Operand);
+
+  // Collect and clone all the compile units referenced from the instructions in
+  // the function (e.g. as a scope).
+  collectDebugInfoFromInstructions(*OldFunc, DIFinder);
   for (auto *Unit : DIFinder.compile_units()) {
     MDNode *MappedUnit =
         MapMetadata(Unit, VMap, RF_None, TypeMapper, Materializer);

[felipepiovezan]

FYI the check in CloneFunctionInto is:

  if (Changes != CloneFunctionChangeType::DifferentModule)
    return;

Should we make it < instead?

[artempyanykh]

@felipepiovezan sorry I didn't get that. Could you elaborate please?

[artempyanykh]

Given that FindDebugInfoToIdentityMap and CollectDebugInfoForCloning get removed later in the stack, I'm going to go ahead and merge this. Please, let me know if I missed something important and I'll fix it on top.