Skip to content

Commit 726ffd3

Browse files
artempyanykhartempyanykh
authored
[NFC][Cloning] Replace IdentityMD set with a predicate in ValueMapper (llvm#129147)
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
1 parent 98c279a commit 726ffd3

File tree

5 files changed

+34
-29
lines changed

5 files changed

+34
-29
lines changed

‎llvm/include/llvm/Transforms/Utils/Cloning.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ void CloneFunctionMetadataInto(Function &NewFunc, const Function &OldFunc,
194194
ValueToValueMapTy &VMap, RemapFlags RemapFlag,
195195
ValueMapTypeRemapper *TypeMapper = nullptr,
196196
ValueMaterializer *Materializer = nullptr,
197-
const MetadataSetTy *IdentityMD = nullptr);
197+
const MetadataPredicate *IdentityMD = nullptr);
198198

199199
/// Clone OldFunc's body into NewFunc.
200200
void CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc,
@@ -204,7 +204,7 @@ void CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc,
204204
ClonedCodeInfo *CodeInfo = nullptr,
205205
ValueMapTypeRemapper *TypeMapper = nullptr,
206206
ValueMaterializer *Materializer = nullptr,
207-
const MetadataSetTy *IdentityMD = nullptr);
207+
const MetadataPredicate *IdentityMD = nullptr);
208208

209209
void CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc,
210210
const Instruction *StartingInst,

‎llvm/include/llvm/Transforms/Utils/ValueMapper.h

+14-13
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class Value;
3737
using ValueToValueMapTy = ValueMap<const Value *, WeakTrackingVH>;
3838
using DbgRecordIterator = simple_ilist<DbgRecord>::iterator;
3939
using MetadataSetTy = SmallPtrSet<const Metadata *, 16>;
40+
using MetadataPredicate = std::function<bool(const Metadata *)>;
4041

4142
/// This is a class that can be implemented by clients to remap types when
4243
/// cloning constants and instructions.
@@ -138,8 +139,8 @@ inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) {
138139
/// alternate \a ValueToValueMapTy and \a ValueMaterializer and returns a ID to
139140
/// pass into the schedule*() functions.
140141
///
141-
/// If an \a IdentityMD set is optionally provided, \a Metadata inside this set
142-
/// will be mapped onto itself in \a VM on first use.
142+
/// If an \a IdentityMD predicate is optionally provided, \a Metadata for which
143+
/// the predicate returns true will be mapped onto itself in \a VM on first use.
143144
///
144145
/// TODO: lib/Linker really doesn't need the \a ValueHandle in the \a
145146
/// ValueToValueMapTy. We should template \a ValueMapper (and its
@@ -158,7 +159,7 @@ class ValueMapper {
158159
ValueMapper(ValueToValueMapTy &VM, RemapFlags Flags = RF_None,
159160
ValueMapTypeRemapper *TypeMapper = nullptr,
160161
ValueMaterializer *Materializer = nullptr,
161-
const MetadataSetTy *IdentityMD = nullptr);
162+
const MetadataPredicate *IdentityMD = nullptr);
162163
ValueMapper(ValueMapper &&) = delete;
163164
ValueMapper(const ValueMapper &) = delete;
164165
ValueMapper &operator=(ValueMapper &&) = delete;
@@ -225,7 +226,7 @@ inline Value *MapValue(const Value *V, ValueToValueMapTy &VM,
225226
RemapFlags Flags = RF_None,
226227
ValueMapTypeRemapper *TypeMapper = nullptr,
227228
ValueMaterializer *Materializer = nullptr,
228-
const MetadataSetTy *IdentityMD = nullptr) {
229+
const MetadataPredicate *IdentityMD = nullptr) {
229230
return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
230231
.mapValue(*V);
231232
}
@@ -239,8 +240,8 @@ inline Value *MapValue(const Value *V, ValueToValueMapTy &VM,
239240
/// \c MD.
240241
/// 3. Else if \c MD is a \a ConstantAsMetadata, call \a MapValue() and
241242
/// re-wrap its return (returning nullptr on nullptr).
242-
/// 4. Else if \c MD is in \c IdentityMD then add an identity mapping for it
243-
/// and return it.
243+
/// 4. Else if \c IdentityMD predicate returns true for \c MD then add an
244+
/// identity mapping for it and return it.
244245
/// 5. Else, \c MD is an \a MDNode. These are remapped, along with their
245246
/// transitive operands. Distinct nodes are duplicated or moved depending
246247
/// on \a RF_MoveDistinctNodes. Uniqued nodes are remapped like constants.
@@ -251,7 +252,7 @@ inline Metadata *MapMetadata(const Metadata *MD, ValueToValueMapTy &VM,
251252
RemapFlags Flags = RF_None,
252253
ValueMapTypeRemapper *TypeMapper = nullptr,
253254
ValueMaterializer *Materializer = nullptr,
254-
const MetadataSetTy *IdentityMD = nullptr) {
255+
const MetadataPredicate *IdentityMD = nullptr) {
255256
return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
256257
.mapMetadata(*MD);
257258
}
@@ -261,7 +262,7 @@ inline MDNode *MapMetadata(const MDNode *MD, ValueToValueMapTy &VM,
261262
RemapFlags Flags = RF_None,
262263
ValueMapTypeRemapper *TypeMapper = nullptr,
263264
ValueMaterializer *Materializer = nullptr,
264-
const MetadataSetTy *IdentityMD = nullptr) {
265+
const MetadataPredicate *IdentityMD = nullptr) {
265266
return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
266267
.mapMDNode(*MD);
267268
}
@@ -278,7 +279,7 @@ inline void RemapInstruction(Instruction *I, ValueToValueMapTy &VM,
278279
RemapFlags Flags = RF_None,
279280
ValueMapTypeRemapper *TypeMapper = nullptr,
280281
ValueMaterializer *Materializer = nullptr,
281-
const MetadataSetTy *IdentityMD = nullptr) {
282+
const MetadataPredicate *IdentityMD = nullptr) {
282283
ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
283284
.remapInstruction(*I);
284285
}
@@ -289,7 +290,7 @@ inline void RemapDbgRecord(Module *M, DbgRecord *DR, ValueToValueMapTy &VM,
289290
RemapFlags Flags = RF_None,
290291
ValueMapTypeRemapper *TypeMapper = nullptr,
291292
ValueMaterializer *Materializer = nullptr,
292-
const MetadataSetTy *IdentityMD = nullptr) {
293+
const MetadataPredicate *IdentityMD = nullptr) {
293294
ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
294295
.remapDbgRecord(M, *DR);
295296
}
@@ -302,7 +303,7 @@ inline void RemapDbgRecordRange(Module *M,
302303
RemapFlags Flags = RF_None,
303304
ValueMapTypeRemapper *TypeMapper = nullptr,
304305
ValueMaterializer *Materializer = nullptr,
305-
const MetadataSetTy *IdentityMD = nullptr) {
306+
const MetadataPredicate *IdentityMD = nullptr) {
306307
ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
307308
.remapDbgRecordRange(M, Range);
308309
}
@@ -317,7 +318,7 @@ inline void RemapFunction(Function &F, ValueToValueMapTy &VM,
317318
RemapFlags Flags = RF_None,
318319
ValueMapTypeRemapper *TypeMapper = nullptr,
319320
ValueMaterializer *Materializer = nullptr,
320-
const MetadataSetTy *IdentityMD = nullptr) {
321+
const MetadataPredicate *IdentityMD = nullptr) {
321322
ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD).remapFunction(F);
322323
}
323324

@@ -326,7 +327,7 @@ inline Constant *MapValue(const Constant *V, ValueToValueMapTy &VM,
326327
RemapFlags Flags = RF_None,
327328
ValueMapTypeRemapper *TypeMapper = nullptr,
328329
ValueMaterializer *Materializer = nullptr,
329-
const MetadataSetTy *IdentityMD = nullptr) {
330+
const MetadataPredicate *IdentityMD = nullptr) {
330331
return ValueMapper(VM, Flags, TypeMapper, Materializer, IdentityMD)
331332
.mapConstant(*V);
332333
}

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -921,11 +921,14 @@ void coro::BaseCloner::create() {
921921
auto savedLinkage = NewF->getLinkage();
922922
NewF->setLinkage(llvm::GlobalValue::ExternalLinkage);
923923

924+
MetadataPredicate IdentityMD = [&](const Metadata *MD) {
925+
return CommonDebugInfo.contains(MD);
926+
};
924927
CloneFunctionAttributesInto(NewF, &OrigF, VMap, false);
925928
CloneFunctionMetadataInto(*NewF, OrigF, VMap, RF_None, nullptr, nullptr,
926-
&CommonDebugInfo);
929+
&IdentityMD);
927930
CloneFunctionBodyInto(*NewF, OrigF, VMap, RF_None, Returns, "", nullptr,
928-
nullptr, nullptr, &CommonDebugInfo);
931+
nullptr, nullptr, &IdentityMD);
929932

930933
auto &Context = NewF->getContext();
931934

‎llvm/lib/Transforms/Utils/CloneFunction.cpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ void llvm::CloneFunctionMetadataInto(Function &NewFunc, const Function &OldFunc,
204204
RemapFlags RemapFlag,
205205
ValueMapTypeRemapper *TypeMapper,
206206
ValueMaterializer *Materializer,
207-
const MetadataSetTy *IdentityMD) {
207+
const MetadataPredicate *IdentityMD) {
208208
SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
209209
OldFunc.getAllMetadata(MDs);
210210
for (auto MD : MDs) {
@@ -221,7 +221,7 @@ void llvm::CloneFunctionBodyInto(Function &NewFunc, const Function &OldFunc,
221221
ClonedCodeInfo *CodeInfo,
222222
ValueMapTypeRemapper *TypeMapper,
223223
ValueMaterializer *Materializer,
224-
const MetadataSetTy *IdentityMD) {
224+
const MetadataPredicate *IdentityMD) {
225225
if (OldFunc.isDeclaration())
226226
return;
227227

@@ -328,8 +328,10 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
328328
DISubprogram *SPClonedWithinModule =
329329
CollectDebugInfoForCloning(*OldFunc, Changes, DIFinder);
330330

331-
MetadataSetTy IdentityMD =
332-
FindDebugInfoToIdentityMap(Changes, DIFinder, SPClonedWithinModule);
331+
MetadataPredicate IdentityMD =
332+
[MDSet =
333+
FindDebugInfoToIdentityMap(Changes, DIFinder, SPClonedWithinModule)](
334+
const Metadata *MD) { return MDSet.contains(MD); };
333335

334336
// Cloning is always a Module level operation, since Metadata needs to be
335337
// cloned.

‎llvm/lib/Transforms/Utils/ValueMapper.cpp

+7-8
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,12 @@ class Mapper {
120120
SmallVector<WorklistEntry, 4> Worklist;
121121
SmallVector<DelayedBasicBlock, 1> DelayedBBs;
122122
SmallVector<Constant *, 16> AppendingInits;
123-
const MetadataSetTy *IdentityMD;
123+
const MetadataPredicate *IdentityMD;
124124

125125
public:
126126
Mapper(ValueToValueMapTy &VM, RemapFlags Flags,
127127
ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer,
128-
const MetadataSetTy *IdentityMD)
128+
const MetadataPredicate *IdentityMD)
129129
: Flags(Flags), TypeMapper(TypeMapper),
130130
MCs(1, MappingContext(VM, Materializer)), IdentityMD(IdentityMD) {}
131131

@@ -904,11 +904,10 @@ std::optional<Metadata *> Mapper::mapSimpleMetadata(const Metadata *MD) {
904904
return wrapConstantAsMetadata(*CMD, mapValue(CMD->getValue()));
905905
}
906906

907-
// Map metadata from IdentityMD on first use. We need to add these nodes to
908-
// the mapping as otherwise metadata nodes numbering gets messed up. This is
909-
// still economical because the amount of data in IdentityMD may be a lot
910-
// larger than what will actually get used.
911-
if (IdentityMD && IdentityMD->contains(MD))
907+
// Map metadata matching IdentityMD predicate on first use. We need to add
908+
// these nodes to the mapping as otherwise metadata nodes numbering gets
909+
// messed up.
910+
if (IdentityMD && (*IdentityMD)(MD))
912911
return getVM().MD()[MD] = TrackingMDRef(const_cast<Metadata *>(MD));
913912

914913
assert(isa<MDNode>(MD) && "Expected a metadata node");
@@ -1211,7 +1210,7 @@ class FlushingMapper {
12111210
ValueMapper::ValueMapper(ValueToValueMapTy &VM, RemapFlags Flags,
12121211
ValueMapTypeRemapper *TypeMapper,
12131212
ValueMaterializer *Materializer,
1214-
const MetadataSetTy *IdentityMD)
1213+
const MetadataPredicate *IdentityMD)
12151214
: pImpl(new Mapper(VM, Flags, TypeMapper, Materializer, IdentityMD)) {}
12161215

12171216
ValueMapper::~ValueMapper() { delete getAsMapper(pImpl); }

0 commit comments

Comments
 (0)