[llvm] [NFC][Coro] Remove now unused CommonDebugInfo in CoroSplit (PR #129150)
Artem Pianykh via llvm-commits
llvm-commits at lists.llvm.org
Sun Mar 9 07:32:44 PDT 2025
https://github.com/artempyanykh updated https://github.com/llvm/llvm-project/pull/129150
>From 336c0203871da8d76dc20dd8fdd6c2e2a8cd52f6 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 09:55:02 -0800
Subject: [PATCH 1/8] [NFC][Cloning] Make ClonedModule case more obvious in
CollectDebugInfoForCloning
Summary:
The code's behavior is unchanged, but it's more obvious right now.
Test Plan:
ninja check-llvm-unit check-llvm
stack-info: PR: https://github.com/llvm/llvm-project/pull/129143, branch: users/artempyanykh/fast-coro-upstream-part2-take2/1
---
llvm/lib/Transforms/Utils/CloneFunction.cpp | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 58d400ac396be..9267930027c04 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -135,6 +135,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)
+ return nullptr;
+
DISubprogram *SPClonedWithinModule = nullptr;
if (Changes < CloneFunctionChangeType::DifferentModule) {
SPClonedWithinModule = F.getSubprogram();
@@ -143,7 +147,7 @@ DISubprogram *llvm::CollectDebugInfoForCloning(const Function &F,
DIFinder.processSubprogram(SPClonedWithinModule);
const Module *M = F.getParent();
- if (Changes != CloneFunctionChangeType::ClonedModule && M) {
+ if (M) {
// Inspect instructions to process e.g. DILexicalBlocks of inlined functions
for (const auto &I : instructions(F))
DIFinder.processInstruction(*M, I);
>From 7888809b93cf57d37c5517016c5206314a94bc42 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 10:29:56 -0800
Subject: [PATCH 2/8] [NFC][Cloning] Simplify the flow in
FindDebugInfoToIdentityMap
Summary:
The new flow should make it more clear what is happening in cases of
Different of Cloned modules.
Test Plan:
ninja check-llvm-unit check-llvm
stack-info: PR: https://github.com/llvm/llvm-project/pull/129144, branch: users/artempyanykh/fast-coro-upstream-part2-take2/2
---
llvm/lib/Transforms/Utils/CloneFunction.cpp | 34 ++++++++++-----------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 9267930027c04..7a309f7390c77 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -160,32 +160,32 @@ MetadataSetTy
llvm::FindDebugInfoToIdentityMap(CloneFunctionChangeType Changes,
DebugInfoFinder &DIFinder,
DISubprogram *SPClonedWithinModule) {
+ if (Changes >= CloneFunctionChangeType::DifferentModule)
+ return {};
+
+ if (DIFinder.subprogram_count() == 0)
+ assert(!SPClonedWithinModule &&
+ "Subprogram should be in DIFinder->subprogram_count()...");
+
MetadataSetTy MD;
- if (Changes < CloneFunctionChangeType::DifferentModule &&
- DIFinder.subprogram_count() > 0) {
- // Avoid cloning types, compile units, and (other) subprograms.
- for (DISubprogram *ISP : DIFinder.subprograms()) {
- if (ISP != SPClonedWithinModule)
- MD.insert(ISP);
- }
+ // Avoid cloning types, compile units, and (other) subprograms.
+ for (DISubprogram *ISP : DIFinder.subprograms())
+ if (ISP != SPClonedWithinModule)
+ MD.insert(ISP);
- // If a subprogram isn't going to be cloned skip its lexical blocks as well.
- for (DIScope *S : DIFinder.scopes()) {
- auto *LScope = dyn_cast<DILocalScope>(S);
- if (LScope && LScope->getSubprogram() != SPClonedWithinModule)
- MD.insert(S);
- }
+ // If a subprogram isn't going to be cloned skip its lexical blocks as well.
+ for (DIScope *S : DIFinder.scopes()) {
+ auto *LScope = dyn_cast<DILocalScope>(S);
+ if (LScope && LScope->getSubprogram() != SPClonedWithinModule)
+ MD.insert(S);
+ }
for (DICompileUnit *CU : DIFinder.compile_units())
MD.insert(CU);
for (DIType *Type : DIFinder.types())
MD.insert(Type);
- } else {
- assert(!SPClonedWithinModule &&
- "Subprogram should be in DIFinder->subprogram_count()...");
- }
return MD;
}
>From 33b79c9d3b7432b693810656a8eec5a17b99c396 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 10:03:41 -0800
Subject: [PATCH 3/8] [NFC][Cloning] Add a helper to collect debug info from
instructions
Summary:
Just moving around. This helper will be used for further refactoring.
Test Plan:
ninja check-llvm-unit check-llvm
stack-info: PR: https://github.com/llvm/llvm-project/pull/129145, branch: users/artempyanykh/fast-coro-upstream-part2-take2/3
---
llvm/lib/Transforms/Utils/CloneFunction.cpp | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 7a309f7390c77..e03c5c27b5ac1 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -40,6 +40,18 @@ using namespace llvm;
#define DEBUG_TYPE "clone-function"
+namespace {
+void collectDebugInfoFromInstructions(const Function &F,
+ DebugInfoFinder &DIFinder) {
+ const Module *M = F.getParent();
+ if (M) {
+ // Inspect instructions to process e.g. DILexicalBlocks of inlined functions
+ for (const auto &I : instructions(F))
+ DIFinder.processInstruction(*M, I);
+ }
+}
+} // namespace
+
/// See comments in Cloning.h.
BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB, ValueToValueMapTy &VMap,
const Twine &NameSuffix, Function *F,
@@ -146,12 +158,7 @@ DISubprogram *llvm::CollectDebugInfoForCloning(const Function &F,
if (SPClonedWithinModule)
DIFinder.processSubprogram(SPClonedWithinModule);
- const Module *M = F.getParent();
- if (M) {
- // Inspect instructions to process e.g. DILexicalBlocks of inlined functions
- for (const auto &I : instructions(F))
- DIFinder.processInstruction(*M, I);
- }
+ collectDebugInfoFromInstructions(F, DIFinder);
return SPClonedWithinModule;
}
>From 6e8df0d2e0b29c1f35f2ef72d2a46349ff468259 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 10:20:06 -0800
Subject: [PATCH 4/8] [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
stack-info: PR: https://github.com/llvm/llvm-project/pull/129146, branch: users/artempyanykh/fast-coro-upstream-part2-take2/4
---
llvm/lib/Transforms/Utils/CloneFunction.cpp | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index e03c5c27b5ac1..dd1b4fe718053 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);
>From b5f28bb5bd02c5607e073352f28e47e04340da12 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 10:53:57 -0800
Subject: [PATCH 5/8] [NFC][Cloning] Replace IdentityMD set with a predicate in
ValueMapper
Summary:
We used the set only to check if it contains certain metadata nodes.
Replacing the set with a predicate makes the intention clearer and the
API more general.
Test Plan:
ninja check-all
stack-info: PR: https://github.com/llvm/llvm-project/pull/129147, branch: users/artempyanykh/fast-coro-upstream-part2-take2/5
---
llvm/include/llvm/Transforms/Utils/Cloning.h | 4 +--
.../llvm/Transforms/Utils/ValueMapper.h | 27 ++++++++++---------
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 7 +++--
llvm/lib/Transforms/Utils/CloneFunction.cpp | 10 ++++---
llvm/lib/Transforms/Utils/ValueMapper.cpp | 15 +++++------
5 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h
index d36f91416db88..2252dda0b9aad 100644
--- a/llvm/include/llvm/Transforms/Utils/Cloning.h
+++ b/llvm/include/llvm/Transforms/Utils/Cloning.h
@@ -194,7 +194,7 @@ void CloneFunctionMetadataInto(Function &NewFunc, const Function &OldFunc,
ValueToValueMapTy &VMap, RemapFlags RemapFlag,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr);
+ const MetadataPredicate *IdentityMD = nullptr);
/// Clone OldFunc's body into NewFunc.
void CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc,
@@ -204,7 +204,7 @@ void CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc,
ClonedCodeInfo *CodeInfo = nullptr,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr);
+ const MetadataPredicate *IdentityMD = nullptr);
void CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc,
const Instruction *StartingInst,
diff --git a/llvm/include/llvm/Transforms/Utils/ValueMapper.h b/llvm/include/llvm/Transforms/Utils/ValueMapper.h
index 852d7095d1133..560df1d3f7f29 100644
--- a/llvm/include/llvm/Transforms/Utils/ValueMapper.h
+++ b/llvm/include/llvm/Transforms/Utils/ValueMapper.h
@@ -37,6 +37,7 @@ class Value;
using ValueToValueMapTy = ValueMap<const Value *, WeakTrackingVH>;
using DbgRecordIterator = simple_ilist<DbgRecord>::iterator;
using MetadataSetTy = SmallPtrSet<const Metadata *, 16>;
+using MetadataPredicate = std::function<bool(const Metadata *)>;
/// This is a class that can be implemented by clients to remap types when
/// cloning constants and instructions.
@@ -138,8 +139,8 @@ inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) {
/// alternate \a ValueToValueMapTy and \a ValueMaterializer and returns a ID to
/// pass into the schedule*() functions.
///
-/// If an \a IdentityMD set is optionally provided, \a Metadata inside this set
-/// will be mapped onto itself in \a VM on first use.
+/// If an \a IdentityMD predicate is optionally provided, \a Metadata for which
+/// the predicate returns true will be mapped onto itself in \a VM on first use.
///
/// TODO: lib/Linker really doesn't need the \a ValueHandle in the \a
/// ValueToValueMapTy. We should template \a ValueMapper (and its
@@ -158,7 +159,7 @@ class ValueMapper {
ValueMapper(ValueToValueMapTy &VM, RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr);
+ const MetadataPredicate *IdentityMD = nullptr);
ValueMapper(ValueMapper &&) = delete;
ValueMapper(const ValueMapper &) = delete;
ValueMapper &operator=(ValueMapper &&) = delete;
@@ -225,7 +226,7 @@ inline Value *MapValue(const Value *V, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr) {
+ const MetadataPredicate *IdentityMD = nullptr) {
return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
.mapValue(*V);
}
@@ -239,8 +240,8 @@ inline Value *MapValue(const Value *V, ValueToValueMapTy &VM,
/// \c MD.
/// 3. Else if \c MD is a \a ConstantAsMetadata, call \a MapValue() and
/// re-wrap its return (returning nullptr on nullptr).
-/// 4. Else if \c MD is in \c IdentityMD then add an identity mapping for it
-/// and return it.
+/// 4. Else if \c IdentityMD predicate returns true for \c MD then add an
+/// identity mapping for it and return it.
/// 5. Else, \c MD is an \a MDNode. These are remapped, along with their
/// transitive operands. Distinct nodes are duplicated or moved depending
/// on \a RF_MoveDistinctNodes. Uniqued nodes are remapped like constants.
@@ -251,7 +252,7 @@ inline Metadata *MapMetadata(const Metadata *MD, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr) {
+ const MetadataPredicate *IdentityMD = nullptr) {
return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
.mapMetadata(*MD);
}
@@ -261,7 +262,7 @@ inline MDNode *MapMetadata(const MDNode *MD, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr) {
+ const MetadataPredicate *IdentityMD = nullptr) {
return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
.mapMDNode(*MD);
}
@@ -278,7 +279,7 @@ inline void RemapInstruction(Instruction *I, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr) {
+ const MetadataPredicate *IdentityMD = nullptr) {
ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
.remapInstruction(*I);
}
@@ -289,7 +290,7 @@ inline void RemapDbgRecord(Module *M, DbgRecord *DR, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr) {
+ const MetadataPredicate *IdentityMD = nullptr) {
ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
.remapDbgRecord(M, *DR);
}
@@ -302,7 +303,7 @@ inline void RemapDbgRecordRange(Module *M,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr) {
+ const MetadataPredicate *IdentityMD = nullptr) {
ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
.remapDbgRecordRange(M, Range);
}
@@ -317,7 +318,7 @@ inline void RemapFunction(Function &F, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr) {
+ const MetadataPredicate *IdentityMD = nullptr) {
ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD).remapFunction(F);
}
@@ -326,7 +327,7 @@ inline Constant *MapValue(const Constant *V, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr,
- const MetadataSetTy *IdentityMD = nullptr) {
+ const MetadataPredicate *IdentityMD = nullptr) {
return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
.mapConstant(*V);
}
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index be35ef99261ed..b2c4e64319725 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -921,11 +921,14 @@ 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,
- &CommonDebugInfo);
+ &IdentityMD);
CloneFunctionBodyInto(*NewF, OrigF, VMap, RF_None, Returns, "", nullptr,
- nullptr, nullptr, &CommonDebugInfo);
+ nullptr, nullptr, &IdentityMD);
auto &Context = NewF->getContext();
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index dd1b4fe718053..502c4898c5940 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -204,7 +204,7 @@ void llvm::CloneFunctionMetadataInto(Function &NewFunc, const Function &OldFunc,
RemapFlags RemapFlag,
ValueMapTypeRemapper *TypeMapper,
ValueMaterializer *Materializer,
- const MetadataSetTy *IdentityMD) {
+ const MetadataPredicate *IdentityMD) {
SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
OldFunc.getAllMetadata(MDs);
for (auto MD : MDs) {
@@ -221,7 +221,7 @@ void llvm::CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc,
ClonedCodeInfo *CodeInfo,
ValueMapTypeRemapper *TypeMapper,
ValueMaterializer *Materializer,
- const MetadataSetTy *IdentityMD) {
+ const MetadataPredicate *IdentityMD) {
if (OldFunc.isDeclaration())
return;
@@ -328,8 +328,10 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
DISubprogram *SPClonedWithinModule =
CollectDebugInfoForCloning(*OldFunc, Changes, DIFinder);
- MetadataSetTy IdentityMD =
- FindDebugInfoToIdentityMap(Changes, DIFinder, SPClonedWithinModule);
+ MetadataPredicate IdentityMD =
+ [MDSet =
+ FindDebugInfoToIdentityMap(Changes, DIFinder, SPClonedWithinModule)](
+ const Metadata *MD) { return MDSet.contains(MD); };
// Cloning is always a Module level operation, since Metadata needs to be
// cloned.
diff --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp
index 9882f81b759ac..5e50536a99206 100644
--- a/llvm/lib/Transforms/Utils/ValueMapper.cpp
+++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp
@@ -120,12 +120,12 @@ class Mapper {
SmallVector<WorklistEntry, 4> Worklist;
SmallVector<DelayedBasicBlock, 1> DelayedBBs;
SmallVector<Constant *, 16> AppendingInits;
- const MetadataSetTy *IdentityMD;
+ const MetadataPredicate *IdentityMD;
public:
Mapper(ValueToValueMapTy &VM, RemapFlags Flags,
ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer,
- const MetadataSetTy *IdentityMD)
+ const MetadataPredicate *IdentityMD)
: Flags(Flags), TypeMapper(TypeMapper),
MCs(1, MappingContext(VM, Materializer)), IdentityMD(IdentityMD) {}
@@ -904,11 +904,10 @@ std::optional<Metadata *> Mapper::mapSimpleMetadata(const Metadata *MD) {
return wrapConstantAsMetadata(*CMD, mapValue(CMD->getValue()));
}
- // Map metadata from IdentityMD on first use. We need to add these nodes to
- // the mapping as otherwise metadata nodes numbering gets messed up. This is
- // still economical because the amount of data in IdentityMD may be a lot
- // larger than what will actually get used.
- if (IdentityMD && IdentityMD->contains(MD))
+ // Map metadata matching IdentityMD predicate on first use. We need to add
+ // these nodes to the mapping as otherwise metadata nodes numbering gets
+ // messed up.
+ if (IdentityMD && (*IdentityMD)(MD))
return getVM().MD()[MD] = TrackingMDRef(const_cast<Metadata *>(MD));
assert(isa<MDNode>(MD) && "Expected a metadata node");
@@ -1211,7 +1210,7 @@ class FlushingMapper {
ValueMapper::ValueMapper(ValueToValueMapTy &VM, RemapFlags Flags,
ValueMapTypeRemapper *TypeMapper,
ValueMaterializer *Materializer,
- const MetadataSetTy *IdentityMD)
+ const MetadataPredicate *IdentityMD)
: pImpl(new Mapper(VM, Flags, TypeMapper, Materializer, IdentityMD)) {}
ValueMapper::~ValueMapper() { delete getAsMapper(pImpl); }
>From a73e948651708ed5dc171729328b46129380d443 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 6/8] [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..8080dca09be00 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.
>From f71d6b8ea90fbad66154d6b82e9b77fc9f08eb16 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 7/8] [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 e0ef90de9c38f627a134c0b18af2e36aafffaf1a 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 8/8] [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