[llvm] [NFC][Cloning] Move DebugInfoFinder decl closer to its place of usage (PR #129154)
Artem Pianykh via llvm-commits
llvm-commits at lists.llvm.org
Sun Mar 9 07:32:47 PDT 2025
https://github.com/artempyanykh updated https://github.com/llvm/llvm-project/pull/129154
>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 01/12] [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 02/12] [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 03/12] [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 04/12] [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 05/12] [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 06/12] [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 07/12] [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 08/12] [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);
}
}
>From 0ca9397687d5e88908fb0481f7e186c6984c0ff2 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 13:00:47 -0800
Subject: [PATCH 09/12] [NFC][Cloning] Remove now unused
FindDebugInfoToIdentityMap
Summary:
This function is no longer needed.
Test Plan:
ninja check-llvm-unit
stack-info: PR: https://github.com/llvm/llvm-project/pull/129151, branch: users/artempyanykh/fast-coro-upstream-part2-take2/9
---
llvm/include/llvm/Transforms/Utils/Cloning.h | 19 -----------
llvm/lib/Transforms/Utils/CloneFunction.cpp | 34 --------------------
2 files changed, 53 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h
index 2252dda0b9aad..ae00c16e7eada 100644
--- a/llvm/include/llvm/Transforms/Utils/Cloning.h
+++ b/llvm/include/llvm/Transforms/Utils/Cloning.h
@@ -244,25 +244,6 @@ DISubprogram *CollectDebugInfoForCloning(const Function &F,
CloneFunctionChangeType Changes,
DebugInfoFinder &DIFinder);
-/// Based on \p Changes and \p DIFinder return debug info that needs to be
-/// identity mapped during Metadata cloning.
-///
-/// NOTE: Such \a MetadataSetTy can be used by \a CloneFunction* to directly
-/// specify metadata that should be identity mapped (and hence not cloned). The
-/// metadata will be identity mapped in \a ValueToValueMapTy on first use. There
-/// are several reasons for doing it this way rather than eagerly identity
-/// mapping metadata nodes in a \a ValueMap:
-/// 1. Mapping metadata is not cheap, particularly because of tracking.
-/// 2. When cloning a Function we identity map lots of global module-level
-/// metadata to avoid cloning it, while only a fraction of it is actually
-/// used by the function. Mapping on first use is a lot faster for modules
-/// with meaningful amount of debug info.
-/// 3. Eagerly identity mapping metadata makes it harder to cache module-level
-/// data (e.g. a set of metadata nodes in a \a DICompileUnit).
-MetadataSetTy FindDebugInfoToIdentityMap(CloneFunctionChangeType Changes,
- DebugInfoFinder &DIFinder,
- DISubprogram *SPClonedWithinModule);
-
/// This class captures the data input to the InlineFunction call, and records
/// the auxiliary results produced by it.
class InlineFunctionInfo {
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 8080dca09be00..11033aeec7dda 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -189,40 +189,6 @@ DISubprogram *llvm::CollectDebugInfoForCloning(const Function &F,
return SPClonedWithinModule;
}
-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;
-
- // 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);
- }
-
- for (DICompileUnit *CU : DIFinder.compile_units())
- MD.insert(CU);
-
- for (DIType *Type : DIFinder.types())
- MD.insert(Type);
-
- return MD;
-}
-
void llvm::CloneFunctionMetadataInto(Function &NewFunc, const Function &OldFunc,
ValueToValueMapTy &VMap,
RemapFlags RemapFlag,
>From c6704005b8d5fa1c0440519cb20f69eafda5ded5 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 13:02:37 -0800
Subject: [PATCH 10/12] [NFC][Cloning] Remove now unused
CollectDebugInfoForCloning
Summary:
This function is no longer used, let's remove it from the header and
impl.
Test Plan:
ninja check-llvm-unit
stack-info: PR: https://github.com/llvm/llvm-project/pull/129152, branch: users/artempyanykh/fast-coro-upstream-part2-take2/10
---
llvm/include/llvm/Transforms/Utils/Cloning.h | 14 -------------
llvm/lib/Transforms/Utils/CloneFunction.cpp | 21 --------------------
2 files changed, 35 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h
index ae00c16e7eada..ec1a1d5faa7e9 100644
--- a/llvm/include/llvm/Transforms/Utils/Cloning.h
+++ b/llvm/include/llvm/Transforms/Utils/Cloning.h
@@ -230,20 +230,6 @@ void CloneAndPruneFunctionInto(Function *NewFunc, const Function *OldFunc,
const char *NameSuffix = "",
ClonedCodeInfo *CodeInfo = nullptr);
-/// Collect debug information such as types, compile units, and other
-/// subprograms that are reachable from \p F and can be considered global for
-/// the purposes of cloning (and hence not needing to be cloned).
-///
-/// What debug information should be processed depends on \p Changes: when
-/// cloning into the same module we process \p F's subprogram and instructions;
-/// when into a cloned module, neither of those.
-///
-/// Returns DISubprogram of the cloned function when cloning into the same
-/// module or nullptr otherwise.
-DISubprogram *CollectDebugInfoForCloning(const Function &F,
- CloneFunctionChangeType Changes,
- DebugInfoFinder &DIFinder);
-
/// This class captures the data input to the InlineFunction call, and records
/// the auxiliary results produced by it.
class InlineFunctionInfo {
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 11033aeec7dda..f32d9454eb076 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -168,27 +168,6 @@ void llvm::CloneFunctionAttributesInto(Function *NewFunc,
OldAttrs.getRetAttrs(), NewArgAttrs));
}
-DISubprogram *llvm::CollectDebugInfoForCloning(const Function &F,
- CloneFunctionChangeType Changes,
- DebugInfoFinder &DIFinder) {
- // 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;
- if (Changes < CloneFunctionChangeType::DifferentModule) {
- SPClonedWithinModule = F.getSubprogram();
- }
- if (SPClonedWithinModule)
- DIFinder.processSubprogram(SPClonedWithinModule);
-
- collectDebugInfoFromInstructions(F, DIFinder);
-
- return SPClonedWithinModule;
-}
-
void llvm::CloneFunctionMetadataInto(Function &NewFunc, const Function &OldFunc,
ValueToValueMapTy &VMap,
RemapFlags RemapFlag,
>From e2e5b39175173ddce8922ae96a477807d5fc41d4 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 13:07:40 -0800
Subject: [PATCH 11/12] [NFC][Cloning] Clean up comments in CloneFunctionInto
Summary:
Some comments no longer make sense nor refer to an existing code path.
Test Plan:
ninja check-llvm-unit
stack-info: PR: https://github.com/llvm/llvm-project/pull/129153, branch: users/artempyanykh/fast-coro-upstream-part2-take2/11
---
llvm/lib/Transforms/Utils/CloneFunction.cpp | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index f32d9454eb076..979cbad0d82c0 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -266,24 +266,13 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
if (OldFunc->isDeclaration())
return;
- // When we remap instructions within the same module, we want to avoid
- // duplicating inlined DISubprograms, so record all subprograms we find as we
- // duplicate instructions and then freeze them in the MD map. We also record
- // information about dbg.value and dbg.declare to avoid duplicating the
- // types.
DebugInfoFinder DIFinder;
- // Track the subprogram attachment that needs to be cloned to fine-tune the
- // mapping within the same module.
if (Changes < CloneFunctionChangeType::DifferentModule) {
- // Need to find subprograms, types, and compile units.
-
assert((NewFunc->getParent() == nullptr ||
NewFunc->getParent() == OldFunc->getParent()) &&
"Expected NewFunc to have the same parent, or no parent");
} else {
- // Need to find all the compile units.
-
assert((NewFunc->getParent() == nullptr ||
NewFunc->getParent() != OldFunc->getParent()) &&
"Expected NewFunc to have different parents, or no parent");
>From 279d02884d97a639cdb28d4ac231bd3f08d824f9 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 25 Feb 2025 13:09:23 -0800
Subject: [PATCH 12/12] [NFC][Cloning] Move DebugInfoFinder decl closer to its
place of usage
Summary:
This makes it clear that DIFinder is only really necessary for llvm.dbg.cu update.
Test Plan:
ninja check-llvm-unit
stack-info: PR: https://github.com/llvm/llvm-project/pull/129154, branch: users/artempyanykh/fast-coro-upstream-part2-take2/12
---
llvm/lib/Transforms/Utils/CloneFunction.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 979cbad0d82c0..3af07594c848b 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -266,8 +266,6 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
if (OldFunc->isDeclaration())
return;
- DebugInfoFinder DIFinder;
-
if (Changes < CloneFunctionChangeType::DifferentModule) {
assert((NewFunc->getParent() == nullptr ||
NewFunc->getParent() == OldFunc->getParent()) &&
@@ -320,7 +318,8 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
Visited.insert(Operand);
// Collect and clone all the compile units referenced from the instructions in
- // the function (e.g. as a scope).
+ // the function (e.g. as instructions' scope).
+ DebugInfoFinder DIFinder;
collectDebugInfoFromInstructions(*OldFunc, DIFinder);
for (auto *Unit : DIFinder.compile_units()) {
MDNode *MappedUnit =
More information about the llvm-commits
mailing list