[llvm] [NFC][Coro] Remove now unused CommonDebugInfo in CoroSplit (PR #129150)
Artem Pianykh via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 13 03:57:26 PDT 2025
https://github.com/artempyanykh updated https://github.com/llvm/llvm-project/pull/129150
>From f235ccffe433f1259700e4a1d70da35c80b45cfc Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 12:07:03 -0800
Subject: [PATCH 1/3] [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:
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: https://github.com/llvm/llvm-project/pull/129148, branch: users/artempyanykh/fast-coro-upstream-part2-take2/6
---
llvm/lib/Transforms/Utils/CloneFunction.cpp | 32 ++++++++++++++++-----
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 502c4898c5940..bdabe0e562fc9 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))
+ 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))
+ 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.
>From 23b20f8d55be51d5cb472119e107e769c19ac099 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 12:42:14 -0800
Subject: [PATCH 2/3] [NFC][Coro] Use CloneFunctionInto for coroutine cloning
instead of CloneFunction<Part>
Summary:
CloneFunctionInto now is fast on its own and we don't need to use
CloneFunctionAttributes/Metadata/Body separately.
CommonDebugInfo in CoroClone is now unused and is cleaned up separately
in the next diff in the stack.
Test Plan:
ninja check-all
stack-info: PR: https://github.com/llvm/llvm-project/pull/129149, branch: users/artempyanykh/fast-coro-upstream-part2-take2/7
---
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index b2c4e64319725..fabbf5f020a74 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -921,14 +921,8 @@ void coro::BaseCloner::create() {
auto savedLinkage = NewF->getLinkage();
NewF->setLinkage(llvm::GlobalValue::ExternalLinkage);
- MetadataPredicate IdentityMD = [&](const Metadata *MD) {
- return CommonDebugInfo.contains(MD);
- };
- CloneFunctionAttributesInto(NewF, &OrigF, VMap, false);
- CloneFunctionMetadataInto(*NewF, OrigF, VMap, RF_None, nullptr, nullptr,
- &IdentityMD);
- CloneFunctionBodyInto(*NewF, OrigF, VMap, RF_None, Returns, "", nullptr,
- nullptr, nullptr, &IdentityMD);
+ CloneFunctionInto(NewF, &OrigF, VMap,
+ CloneFunctionChangeType::LocalChangesOnly, Returns);
auto &Context = NewF->getContext();
>From 08707d65ee541c351ac246a02693a4c7127cebb1 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 12:47:10 -0800
Subject: [PATCH 3/3] [NFC][Coro] Remove now unused CommonDebugInfo in
CoroSplit
Summary:
This cleans up the now unnecessary debug info collection in CoroSplit.
This makes CoroSplit pass almost as fast with -g2 as it is with -g1 on
the sample cpp file used with other parts of this stack:
| | Baseline | IdentityMD set | Prebuilt CommonDI | MetadataPred (cur) |
|-----------------|----------|----------------|-------------------|--------------------|
| CoroSplitPass | 306ms | 221ms | 68ms | 3.8ms |
| CoroCloner | 101ms | 72ms | 0.5ms | 0.5ms |
| CollectCommonDI | - | - | 63ms | - |
| Speed up | 1x | 1.4x | 4.5x | 80x |
Test Plan:
ninja check-all
stack-info: PR: https://github.com/llvm/llvm-project/pull/129150, branch: users/artempyanykh/fast-coro-upstream-part2-take2/8
---
llvm/lib/Transforms/Coroutines/CoroCloner.h | 31 ++++++----------
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 37 +++-----------------
2 files changed, 16 insertions(+), 52 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroCloner.h b/llvm/lib/Transforms/Coroutines/CoroCloner.h
index b817e55cad9fc..d1887980fb3bc 100644
--- a/llvm/lib/Transforms/Coroutines/CoroCloner.h
+++ b/llvm/lib/Transforms/Coroutines/CoroCloner.h
@@ -48,9 +48,6 @@ class BaseCloner {
CloneKind FKind;
IRBuilder<> Builder;
TargetTransformInfo &TTI;
- // Common module-level metadata that's shared between all coroutine clones and
- // doesn't need to be cloned itself.
- const MetadataSetTy &CommonDebugInfo;
ValueToValueMapTy VMap;
Function *NewF = nullptr;
@@ -63,12 +60,12 @@ class BaseCloner {
/// Create a cloner for a continuation lowering.
BaseCloner(Function &OrigF, const Twine &Suffix, coro::Shape &Shape,
Function *NewF, AnyCoroSuspendInst *ActiveSuspend,
- TargetTransformInfo &TTI, const MetadataSetTy &CommonDebugInfo)
+ TargetTransformInfo &TTI)
: OrigF(OrigF), Suffix(Suffix), Shape(Shape),
FKind(Shape.ABI == ABI::Async ? CloneKind::Async
: CloneKind::Continuation),
- Builder(OrigF.getContext()), TTI(TTI), CommonDebugInfo(CommonDebugInfo),
- NewF(NewF), ActiveSuspend(ActiveSuspend) {
+ Builder(OrigF.getContext()), TTI(TTI), NewF(NewF),
+ ActiveSuspend(ActiveSuspend) {
assert(Shape.ABI == ABI::Retcon || Shape.ABI == ABI::RetconOnce ||
Shape.ABI == ABI::Async);
assert(NewF && "need existing function for continuation");
@@ -77,11 +74,9 @@ class BaseCloner {
public:
BaseCloner(Function &OrigF, const Twine &Suffix, coro::Shape &Shape,
- CloneKind FKind, TargetTransformInfo &TTI,
- const MetadataSetTy &CommonDebugInfo)
+ CloneKind FKind, TargetTransformInfo &TTI)
: OrigF(OrigF), Suffix(Suffix), Shape(Shape), FKind(FKind),
- Builder(OrigF.getContext()), TTI(TTI),
- CommonDebugInfo(CommonDebugInfo) {}
+ Builder(OrigF.getContext()), TTI(TTI) {}
virtual ~BaseCloner() {}
@@ -89,14 +84,12 @@ class BaseCloner {
static Function *createClone(Function &OrigF, const Twine &Suffix,
coro::Shape &Shape, Function *NewF,
AnyCoroSuspendInst *ActiveSuspend,
- TargetTransformInfo &TTI,
- const MetadataSetTy &CommonDebugInfo) {
+ TargetTransformInfo &TTI) {
assert(Shape.ABI == ABI::Retcon || Shape.ABI == ABI::RetconOnce ||
Shape.ABI == ABI::Async);
TimeTraceScope FunctionScope("BaseCloner");
- BaseCloner Cloner(OrigF, Suffix, Shape, NewF, ActiveSuspend, TTI,
- CommonDebugInfo);
+ BaseCloner Cloner(OrigF, Suffix, Shape, NewF, ActiveSuspend, TTI);
Cloner.create();
return Cloner.getFunction();
}
@@ -136,9 +129,8 @@ class SwitchCloner : public BaseCloner {
protected:
/// Create a cloner for a switch lowering.
SwitchCloner(Function &OrigF, const Twine &Suffix, coro::Shape &Shape,
- CloneKind FKind, TargetTransformInfo &TTI,
- const MetadataSetTy &CommonDebugInfo)
- : BaseCloner(OrigF, Suffix, Shape, FKind, TTI, CommonDebugInfo) {}
+ CloneKind FKind, TargetTransformInfo &TTI)
+ : BaseCloner(OrigF, Suffix, Shape, FKind, TTI) {}
void create() override;
@@ -146,12 +138,11 @@ class SwitchCloner : public BaseCloner {
/// Create a clone for a switch lowering.
static Function *createClone(Function &OrigF, const Twine &Suffix,
coro::Shape &Shape, CloneKind FKind,
- TargetTransformInfo &TTI,
- const MetadataSetTy &CommonDebugInfo) {
+ TargetTransformInfo &TTI) {
assert(Shape.ABI == ABI::Switch);
TimeTraceScope FunctionScope("SwitchCloner");
- SwitchCloner Cloner(OrigF, Suffix, Shape, FKind, TTI, CommonDebugInfo);
+ SwitchCloner Cloner(OrigF, Suffix, Shape, FKind, TTI);
Cloner.create();
return Cloner.getFunction();
}
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index fabbf5f020a74..f9a6c70fedc2d 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -78,24 +78,6 @@ using namespace llvm;
#define DEBUG_TYPE "coro-split"
-namespace {
-/// Collect (a known) subset of global debug info metadata potentially used by
-/// the function \p F.
-///
-/// This metadata set can be used to avoid cloning debug info not owned by \p F
-/// and is shared among all potential clones \p F.
-MetadataSetTy collectCommonDebugInfo(Function &F) {
- TimeTraceScope FunctionScope("CollectCommonDebugInfo");
-
- DebugInfoFinder DIFinder;
- DISubprogram *SPClonedWithinModule = CollectDebugInfoForCloning(
- F, CloneFunctionChangeType::LocalChangesOnly, DIFinder);
-
- return FindDebugInfoToIdentityMap(CloneFunctionChangeType::LocalChangesOnly,
- DIFinder, SPClonedWithinModule);
-}
-} // end anonymous namespace
-
// FIXME:
// Lower the intrinisc in CoroEarly phase if coroutine frame doesn't escape
// and it is known that other transformations, for example, sanitizers
@@ -1406,21 +1388,16 @@ struct SwitchCoroutineSplitter {
TargetTransformInfo &TTI) {
assert(Shape.ABI == coro::ABI::Switch);
- MetadataSetTy CommonDebugInfo{collectCommonDebugInfo(F)};
-
// Create a resume clone by cloning the body of the original function,
// setting new entry block and replacing coro.suspend an appropriate value
// to force resume or cleanup pass for every suspend point.
createResumeEntryBlock(F, Shape);
auto *ResumeClone = coro::SwitchCloner::createClone(
- F, ".resume", Shape, coro::CloneKind::SwitchResume, TTI,
- CommonDebugInfo);
+ F, ".resume", Shape, coro::CloneKind::SwitchResume, TTI);
auto *DestroyClone = coro::SwitchCloner::createClone(
- F, ".destroy", Shape, coro::CloneKind::SwitchUnwind, TTI,
- CommonDebugInfo);
+ F, ".destroy", Shape, coro::CloneKind::SwitchUnwind, TTI);
auto *CleanupClone = coro::SwitchCloner::createClone(
- F, ".cleanup", Shape, coro::CloneKind::SwitchCleanup, TTI,
- CommonDebugInfo);
+ F, ".cleanup", Shape, coro::CloneKind::SwitchCleanup, TTI);
postSplitCleanup(*ResumeClone);
postSplitCleanup(*DestroyClone);
@@ -1806,14 +1783,12 @@ void coro::AsyncABI::splitCoroutine(Function &F, coro::Shape &Shape,
assert(Clones.size() == Shape.CoroSuspends.size());
- MetadataSetTy CommonDebugInfo{collectCommonDebugInfo(F)};
-
for (auto [Idx, CS] : llvm::enumerate(Shape.CoroSuspends)) {
auto *Suspend = CS;
auto *Clone = Clones[Idx];
coro::BaseCloner::createClone(F, "resume." + Twine(Idx), Shape, Clone,
- Suspend, TTI, CommonDebugInfo);
+ Suspend, TTI);
}
}
@@ -1940,14 +1915,12 @@ void coro::AnyRetconABI::splitCoroutine(Function &F, coro::Shape &Shape,
assert(Clones.size() == Shape.CoroSuspends.size());
- MetadataSetTy CommonDebugInfo{collectCommonDebugInfo(F)};
-
for (auto [Idx, CS] : llvm::enumerate(Shape.CoroSuspends)) {
auto Suspend = CS;
auto Clone = Clones[Idx];
coro::BaseCloner::createClone(F, "resume." + Twine(Idx), Shape, Clone,
- Suspend, TTI, CommonDebugInfo);
+ Suspend, TTI);
}
}
More information about the llvm-commits
mailing list