https://github.com/artempyanykh updated
https://github.com/llvm/llvm-project/pull/129150
>From d0c5ddcf3defff02566f5e41fb93b1689436977d Mon Sep 17 00:00:00 2001
From: Artem Pianykh <a...@fb.com>
Date: Tue, 25 Feb 2025 12:47:10 -0800
Subject: [PATCH] [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);
}
}
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
