Skip to content

Commit 08707d6

Browse files
artempyanykhartempyanykh
committed
[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: #129150, branch: users/artempyanykh/fast-coro-upstream-part2-take2/8
1 parent 23b20f8 commit 08707d6

File tree

2 files changed

+16
-52
lines changed

2 files changed

+16
-52
lines changed

‎llvm/lib/Transforms/Coroutines/CoroCloner.h

+11-20
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,6 @@ class BaseCloner {
4848
CloneKind FKind;
4949
IRBuilder<> Builder;
5050
TargetTransformInfo &TTI;
51-
// Common module-level metadata that's shared between all coroutine clones and
52-
// doesn't need to be cloned itself.
53-
const MetadataSetTy &CommonDebugInfo;
5451

5552
ValueToValueMapTy VMap;
5653
Function *NewF = nullptr;
@@ -63,12 +60,12 @@ class BaseCloner {
6360
/// Create a cloner for a continuation lowering.
6461
BaseCloner(Function &OrigF, const Twine &Suffix, coro::Shape &Shape,
6562
Function *NewF, AnyCoroSuspendInst *ActiveSuspend,
66-
TargetTransformInfo &TTI, const MetadataSetTy &CommonDebugInfo)
63+
TargetTransformInfo &TTI)
6764
: OrigF(OrigF), Suffix(Suffix), Shape(Shape),
6865
FKind(Shape.ABI == ABI::Async ? CloneKind::Async
6966
: CloneKind::Continuation),
70-
Builder(OrigF.getContext()), TTI(TTI), CommonDebugInfo(CommonDebugInfo),
71-
NewF(NewF), ActiveSuspend(ActiveSuspend) {
67+
Builder(OrigF.getContext()), TTI(TTI), NewF(NewF),
68+
ActiveSuspend(ActiveSuspend) {
7269
assert(Shape.ABI == ABI::Retcon || Shape.ABI == ABI::RetconOnce ||
7370
Shape.ABI == ABI::Async);
7471
assert(NewF && "need existing function for continuation");
@@ -77,26 +74,22 @@ class BaseCloner {
7774

7875
public:
7976
BaseCloner(Function &OrigF, const Twine &Suffix, coro::Shape &Shape,
80-
CloneKind FKind, TargetTransformInfo &TTI,
81-
const MetadataSetTy &CommonDebugInfo)
77+
CloneKind FKind, TargetTransformInfo &TTI)
8278
: OrigF(OrigF), Suffix(Suffix), Shape(Shape), FKind(FKind),
83-
Builder(OrigF.getContext()), TTI(TTI),
84-
CommonDebugInfo(CommonDebugInfo) {}
79+
Builder(OrigF.getContext()), TTI(TTI) {}
8580

8681
virtual ~BaseCloner() {}
8782

8883
/// Create a clone for a continuation lowering.
8984
static Function *createClone(Function &OrigF, const Twine &Suffix,
9085
coro::Shape &Shape, Function *NewF,
9186
AnyCoroSuspendInst *ActiveSuspend,
92-
TargetTransformInfo &TTI,
93-
const MetadataSetTy &CommonDebugInfo) {
87+
TargetTransformInfo &TTI) {
9488
assert(Shape.ABI == ABI::Retcon || Shape.ABI == ABI::RetconOnce ||
9589
Shape.ABI == ABI::Async);
9690
TimeTraceScope FunctionScope("BaseCloner");
9791

98-
BaseCloner Cloner(OrigF, Suffix, Shape, NewF, ActiveSuspend, TTI,
99-
CommonDebugInfo);
92+
BaseCloner Cloner(OrigF, Suffix, Shape, NewF, ActiveSuspend, TTI);
10093
Cloner.create();
10194
return Cloner.getFunction();
10295
}
@@ -136,22 +129,20 @@ class SwitchCloner : public BaseCloner {
136129
protected:
137130
/// Create a cloner for a switch lowering.
138131
SwitchCloner(Function &OrigF, const Twine &Suffix, coro::Shape &Shape,
139-
CloneKind FKind, TargetTransformInfo &TTI,
140-
const MetadataSetTy &CommonDebugInfo)
141-
: BaseCloner(OrigF, Suffix, Shape, FKind, TTI, CommonDebugInfo) {}
132+
CloneKind FKind, TargetTransformInfo &TTI)
133+
: BaseCloner(OrigF, Suffix, Shape, FKind, TTI) {}
142134

143135
void create() override;
144136

145137
public:
146138
/// Create a clone for a switch lowering.
147139
static Function *createClone(Function &OrigF, const Twine &Suffix,
148140
coro::Shape &Shape, CloneKind FKind,
149-
TargetTransformInfo &TTI,
150-
const MetadataSetTy &CommonDebugInfo) {
141+
TargetTransformInfo &TTI) {
151142
assert(Shape.ABI == ABI::Switch);
152143
TimeTraceScope FunctionScope("SwitchCloner");
153144

154-
SwitchCloner Cloner(OrigF, Suffix, Shape, FKind, TTI, CommonDebugInfo);
145+
SwitchCloner Cloner(OrigF, Suffix, Shape, FKind, TTI);
155146
Cloner.create();
156147
return Cloner.getFunction();
157148
}

‎llvm/lib/Transforms/Coroutines/CoroSplit.cpp

+5-32
Original file line numberDiff line numberDiff line change
@@ -78,24 +78,6 @@ using namespace llvm;
7878

7979
#define DEBUG_TYPE "coro-split"
8080

81-
namespace {
82-
/// Collect (a known) subset of global debug info metadata potentially used by
83-
/// the function \p F.
84-
///
85-
/// This metadata set can be used to avoid cloning debug info not owned by \p F
86-
/// and is shared among all potential clones \p F.
87-
MetadataSetTy collectCommonDebugInfo(Function &F) {
88-
TimeTraceScope FunctionScope("CollectCommonDebugInfo");
89-
90-
DebugInfoFinder DIFinder;
91-
DISubprogram *SPClonedWithinModule = CollectDebugInfoForCloning(
92-
F, CloneFunctionChangeType::LocalChangesOnly, DIFinder);
93-
94-
return FindDebugInfoToIdentityMap(CloneFunctionChangeType::LocalChangesOnly,
95-
DIFinder, SPClonedWithinModule);
96-
}
97-
} // end anonymous namespace
98-
9981
// FIXME:
10082
// Lower the intrinisc in CoroEarly phase if coroutine frame doesn't escape
10183
// and it is known that other transformations, for example, sanitizers
@@ -1406,21 +1388,16 @@ struct SwitchCoroutineSplitter {
14061388
TargetTransformInfo &TTI) {
14071389
assert(Shape.ABI == coro::ABI::Switch);
14081390

1409-
MetadataSetTy CommonDebugInfo{collectCommonDebugInfo(F)};
1410-
14111391
// Create a resume clone by cloning the body of the original function,
14121392
// setting new entry block and replacing coro.suspend an appropriate value
14131393
// to force resume or cleanup pass for every suspend point.
14141394
createResumeEntryBlock(F, Shape);
14151395
auto *ResumeClone = coro::SwitchCloner::createClone(
1416-
F, ".resume", Shape, coro::CloneKind::SwitchResume, TTI,
1417-
CommonDebugInfo);
1396+
F, ".resume", Shape, coro::CloneKind::SwitchResume, TTI);
14181397
auto *DestroyClone = coro::SwitchCloner::createClone(
1419-
F, ".destroy", Shape, coro::CloneKind::SwitchUnwind, TTI,
1420-
CommonDebugInfo);
1398+
F, ".destroy", Shape, coro::CloneKind::SwitchUnwind, TTI);
14211399
auto *CleanupClone = coro::SwitchCloner::createClone(
1422-
F, ".cleanup", Shape, coro::CloneKind::SwitchCleanup, TTI,
1423-
CommonDebugInfo);
1400+
F, ".cleanup", Shape, coro::CloneKind::SwitchCleanup, TTI);
14241401

14251402
postSplitCleanup(*ResumeClone);
14261403
postSplitCleanup(*DestroyClone);
@@ -1806,14 +1783,12 @@ void coro::AsyncABI::splitCoroutine(Function &F, coro::Shape &Shape,
18061783

18071784
assert(Clones.size() == Shape.CoroSuspends.size());
18081785

1809-
MetadataSetTy CommonDebugInfo{collectCommonDebugInfo(F)};
1810-
18111786
for (auto [Idx, CS] : llvm::enumerate(Shape.CoroSuspends)) {
18121787
auto *Suspend = CS;
18131788
auto *Clone = Clones[Idx];
18141789

18151790
coro::BaseCloner::createClone(F, "resume." + Twine(Idx), Shape, Clone,
1816-
Suspend, TTI, CommonDebugInfo);
1791+
Suspend, TTI);
18171792
}
18181793
}
18191794

@@ -1940,14 +1915,12 @@ void coro::AnyRetconABI::splitCoroutine(Function &F, coro::Shape &Shape,
19401915

19411916
assert(Clones.size() == Shape.CoroSuspends.size());
19421917

1943-
MetadataSetTy CommonDebugInfo{collectCommonDebugInfo(F)};
1944-
19451918
for (auto [Idx, CS] : llvm::enumerate(Shape.CoroSuspends)) {
19461919
auto Suspend = CS;
19471920
auto Clone = Clones[Idx];
19481921

19491922
coro::BaseCloner::createClone(F, "resume." + Twine(Idx), Shape, Clone,
1950-
Suspend, TTI, CommonDebugInfo);
1923+
Suspend, TTI);
19511924
}
19521925
}
19531926

0 commit comments

Comments
 (0)