[NFC][Cloning] Replace DIFinder usage in CloneFunctionInto with a MetadataPredicate #129148

artempyanykh · 2025-02-27T23:34:37Z

Stacked PRs:

[NFC][Cloning] Replace DIFinder usage in CloneFunctionInto with a MetadataPredicate

Summary:
The new code should be functionally identical to the old one (but
faster). The reasoning is as follows.

In the old code when cloning within the module:

DIFinder traverses and collects all debug info reachable from a
function, its instructions, and its owning compile unit.
Then "compile units, types, other subprograms, and lexical blocks of
other subprograms" are saved in a set.
Then when we MapMetadata, we traverse the function's debug info
again and those nodes that are in the set from p.2 are identity
mapped.

This looks equivalent to just doing step 3 with identity mapping based
on a predicate that says to identity map "compile units, types, other
subprograms, and lexical blocks of other subprograms" (same as in step
2). This is what the new code does.

Test Plan:
ninja check-all
There's a bunch of tests around cloning and all of them pass.

llvmbot · 2025-02-27T23:36:15Z

@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-coroutines

@llvm/pr-subscribers-llvm-transforms

Author: Artem Pianykh (artempyanykh)

Changes

Stacked PRs:

#129154
#129153
#129152
#129151
#129150
#129149
->#129148
#129147
#129146
#129145
#129144
#129143

[NFC][Cloning] Replace DIFinder usage in CloneFunctionInto with a MetadataPredicate

Summary:
The new code should be functionally identical to the old one (but
faster). The reasoning is as follows.

In the old code when cloning within the module:

DIFinder traverses and collects all debug info reachable from a
function, its instructions, and its owning compile unit.
Then "compile units, types, other subprograms, and lexical blocks of
other subprograms" are saved in a set.
Then when we MapMetadata, we traverse the function's debug info
again and those nodes that are in the set from p.2 are identity
mapped.

This looks equivalent to just doing step 3 with identity mapping based
on a predicate that says to identity map "compile units, types, other
subprograms, and lexical blocks of other subprograms" (same as in step
2). This is what the new code does.

Test Plan:
ninja check-all
There's a bunch of tests around cloning and all of them pass.

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

1 Files Affected:

(modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+25-7)

diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 8e2a02d49d35c..17c14c09082b0 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -50,6 +50,30 @@ void collectDebugInfoFromInstructions(const Function &F,
       DIFinder.processInstruction(*M, I);
   }
 }
+
+// Create a predicate that matches the metadata that should be identity mapped
+// during function cloning.
+MetadataPredicate createIdentityMDPredicate(const Function &F,
+                                            CloneFunctionChangeType Changes) {
+  if (Changes >= CloneFunctionChangeType::DifferentModule)
+    return [](const Metadata *MD) { return false; };
+
+  DISubprogram *SPClonedWithinModule = F.getSubprogram();
+  return [=](const Metadata *MD) {
+    // Avoid cloning types, compile units, and (other) subprograms.
+    if (isa<DICompileUnit>(MD) || isa<DIType>(MD))
+      return true;
+
+    if (auto *SP = dyn_cast<DISubprogram>(MD); SP)
+      return SP != SPClonedWithinModule;
+
+    // If a subprogram isn't going to be cloned skip its lexical blocks as well.
+    if (auto *LScope = dyn_cast<DILocalScope>(MD); LScope)
+      return LScope->getSubprogram() != SPClonedWithinModule;
+
+    return false;
+  };
+}
 } // namespace
 
 /// See comments in Cloning.h.
@@ -325,13 +349,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
     }
   }
 
-  DISubprogram *SPClonedWithinModule =
-      CollectDebugInfoForCloning(*OldFunc, Changes, DIFinder);
-
-  MetadataPredicate IdentityMD =
-      [MDSet =
-           FindDebugInfoToIdentityMap(Changes, DIFinder, SPClonedWithinModule)](
-          const Metadata *MD) { return MDSet.contains(MD); };
+  MetadataPredicate IdentityMD = createIdentityMDPredicate(*OldFunc, Changes);
 
   // Cloning is always a Module level operation, since Metadata needs to be
   // cloned.

…adataPredicate Summary: The new code should be functionally identical to the old one (but faster). The reasoning is as follows. In the old code when cloning within the module: 1. DIFinder traverses and collects *all* debug info reachable from a function, its instructions, and its owning compile unit. 2. Then "compile units, types, other subprograms, and lexical blocks of other subprograms" are saved in a set. 3. Then when we MapMetadata, we traverse the function's debug info _again_ and those nodes that are in the set from p.2 are identity mapped. This looks equivalent to just doing step 3 with identity mapping based on a predicate that says to identity map "compile units, types, other subprograms, and lexical blocks of other subprograms" (same as in step 2). This is what the new code does. Test Plan: ninja check-all There's a bunch of tests around cloning and all of them pass. stack-info: PR: llvm/llvm-project#129148, branch: users/artempyanykh/fast-coro-upstream-part2-take2/6

felipepiovezan · 2025-03-04T15:40:01Z

llvm/lib/Transforms/Utils/CloneFunction.cpp

+    if (isa<DICompileUnit>(MD) || isa<DIType>(MD))
+      return true;
+
+    if (auto *SP = dyn_cast<DISubprogram>(MD); SP)


nit: the ; SP is redundant here

same with ; LScope below

felipepiovezan

This is pretty neat!

…adataPredicate Summary: The new code should be functionally identical to the old one (but faster). The reasoning is as follows. In the old code when cloning within the module: 1. DIFinder traverses and collects *all* debug info reachable from a function, its instructions, and its owning compile unit. 2. Then "compile units, types, other subprograms, and lexical blocks of other subprograms" are saved in a set. 3. Then when we MapMetadata, we traverse the function's debug info _again_ and those nodes that are in the set from p.2 are identity mapped. This looks equivalent to just doing step 3 with identity mapping based on a predicate that says to identity map "compile units, types, other subprograms, and lexical blocks of other subprograms" (same as in step 2). This is what the new code does. Test Plan: ninja check-all There's a bunch of tests around cloning and all of them pass. stack-info: PR: #129148, branch: users/artempyanykh/fast-coro-upstream-part2-take2/6

llvm-ci · 2025-03-13T17:29:39Z

LLVM Buildbot has detected a new failure on builder ml-opt-devrel-x86-64 running on ml-opt-devrel-x86-64-b1 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/175/builds/14880