Skip to content

[NFC][Cloning] Replace IdentityMD set with a predicate in ValueMapper #129147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 12, 2025
Merged

[NFC][Cloning] Replace IdentityMD set with a predicate in ValueMapper #129147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions llvm/include/llvm/Transforms/Utils/Cloning.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
27 changes: 14 additions & 13 deletions llvm/include/llvm/Transforms/Utils/ValueMapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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.
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}

Expand All @@ -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);
}
Expand Down
7 changes: 5 additions & 2 deletions llvm/lib/Transforms/Coroutines/CoroSplit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
10 changes: 6 additions & 4 deletions llvm/lib/Transforms/Utils/CloneFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
15 changes: 7 additions & 8 deletions llvm/lib/Transforms/Utils/ValueMapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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); }
Expand Down