Skip to content

Commit 133e8d7

Browse files
committed
Add FIXME comments with reference to new issue, and invoke newly created
metadata merge functions for memprof and callsite metadata.
1 parent 3e226cb commit 133e8d7

File tree

6 files changed

+44
-1
lines changed

6 files changed

+44
-1
lines changed

llvm/include/llvm/IR/Metadata.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,6 +1464,8 @@ class MDNode : public Metadata {
14641464
static MDNode *getMergedProfMetadata(MDNode *A, MDNode *B,
14651465
const Instruction *AInstr,
14661466
const Instruction *BInstr);
1467+
static MDNode *getMergedMemProfMetadata(MDNode *A, MDNode *B);
1468+
static MDNode *getMergedCallsiteMetadata(MDNode *A, MDNode *B);
14671469
};
14681470

14691471
/// Tuple of metadata.

llvm/include/llvm/Transforms/Utils/Local.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,11 @@ Instruction *removeUnwindEdge(BasicBlock *BB, DomTreeUpdater *DTU = nullptr);
412412
bool removeUnreachableBlocks(Function &F, DomTreeUpdater *DTU = nullptr,
413413
MemorySSAUpdater *MSSAU = nullptr);
414414

415+
/// DO NOT CALL EXTERNALLY.
416+
/// FIXME: https://github.com/llvm/llvm-project/issues/121495
417+
/// Once external callers of this function are removed, either inline into
418+
/// combineMetadataForCSE, or internalize and remove KnownIDs parameter.
419+
///
415420
/// Combine the metadata of two instructions so that K can replace J. Some
416421
/// metadata kinds can only be kept if K does not move, meaning it dominated
417422
/// J in the original IR.

llvm/lib/Analysis/MemoryProfileInfo.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,3 +347,24 @@ template <> uint64_t CallStack<MDNode, MDNode::op_iterator>::back() const {
347347
return mdconst::dyn_extract<ConstantInt>(N->operands().back())
348348
->getZExtValue();
349349
}
350+
351+
MDNode *MDNode::getMergedMemProfMetadata(MDNode *A, MDNode *B) {
352+
if (!(A && B)) {
353+
return A ? A : B;
354+
}
355+
356+
// TODO: Support more sophisticated merging, such as selecting the one with
357+
// more bytes allocated, or implement support for carrying multiple allocation
358+
// leaf contexts. For now, keep the first one.
359+
return A;
360+
}
361+
362+
MDNode *MDNode::getMergedCallsiteMetadata(MDNode *A, MDNode *B) {
363+
if (!(A && B)) {
364+
return A ? A : B;
365+
}
366+
367+
// TODO: Support more sophisticated merging, which will require support for
368+
// carrying multiple contexts. For now, keep the first one.
369+
return A;
370+
}

llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,9 @@ Instruction *InstCombinerImpl::foldPHIArgLoadIntoPHI(PHINode &PN) {
788788
BasicBlock *BB = std::get<0>(Incoming);
789789
Value *V = std::get<1>(Incoming);
790790
LoadInst *LI = cast<LoadInst>(V);
791+
// FIXME: https://github.com/llvm/llvm-project/issues/121495
792+
// Call combineMetadataForCSE instead, so that an explicit set of KnownIDs
793+
// doesn't need to be maintained here.
791794
combineMetadata(NewLI, LI, KnownIDs, true);
792795
Value *NewInVal = LI->getOperand(0);
793796
if (NewInVal != InVal)

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,9 @@ static void combineAAMetadata(Instruction *ReplInst, Instruction *I) {
350350
LLVMContext::MD_noalias, LLVMContext::MD_invariant_group,
351351
LLVMContext::MD_access_group, LLVMContext::MD_prof,
352352
LLVMContext::MD_memprof, LLVMContext::MD_callsite};
353+
// FIXME: https://github.com/llvm/llvm-project/issues/121495
354+
// Use custom AA metadata combining handling instead of combineMetadata, which
355+
// is meant for CSE and will drop any metadata not in the KnownIDs list.
353356
combineMetadata(ReplInst, I, KnownIDs, true);
354357
}
355358

llvm/lib/Transforms/Utils/Local.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3308,6 +3308,9 @@ bool llvm::removeUnreachableBlocks(Function &F, DomTreeUpdater *DTU,
33083308
return Changed;
33093309
}
33103310

3311+
// FIXME: https://github.com/llvm/llvm-project/issues/121495
3312+
// Once external callers of this function are removed, either inline into
3313+
// combineMetadataForCSE, or internalize and remove KnownIDs parameter.
33113314
void llvm::combineMetadata(Instruction *K, const Instruction *J,
33123315
ArrayRef<unsigned> KnownIDs, bool DoesKMove) {
33133316
SmallVector<std::pair<unsigned, MDNode *>, 4> Metadata;
@@ -3320,6 +3323,10 @@ void llvm::combineMetadata(Instruction *K, const Instruction *J,
33203323

33213324
switch (Kind) {
33223325
default:
3326+
// FIXME: https://github.com/llvm/llvm-project/issues/121495
3327+
// Change to removing only explicitly listed other metadata, and assert
3328+
// on unknown metadata, to avoid inadvertently dropping newly added
3329+
// metadata types.
33233330
K->setMetadata(Kind, nullptr); // Remove unknown metadata
33243331
break;
33253332
case LLVMContext::MD_dbg:
@@ -3380,8 +3387,10 @@ void llvm::combineMetadata(Instruction *K, const Instruction *J,
33803387
MDNode::getMostGenericAlignmentOrDereferenceable(JMD, KMD));
33813388
break;
33823389
case LLVMContext::MD_memprof:
3390+
K->setMetadata(Kind, MDNode::getMergedMemProfMetadata(KMD, JMD));
3391+
break;
33833392
case LLVMContext::MD_callsite:
3384-
// Preserve !memprof and !callsite metadata on K.
3393+
K->setMetadata(Kind, MDNode::getMergedCallsiteMetadata(KMD, JMD));
33853394
break;
33863395
case LLVMContext::MD_preserve_access_index:
33873396
// Preserve !preserve.access.index in K.

0 commit comments

Comments
 (0)