Skip to content

Commit b757d84

Browse files
teresajohnsonaokblast
authored andcommitted
[MemProf] Fix the propagation of context/size info after inlining (llvm#164872)
In certain cases the context/size info we use for reporting of hinted bytes in the LTO link was being dropped when we re-constructed context tries and memprof metadata after inlining. This only affected cases where we were using the -memprof-min-percent-max-cold-size option to only keep that information for the largest cold contexts, and where the pre-LTO compile did *not* specify -memprof-report-hinted-sizes. The issue is that we don't have a MaxSize, which is only available during the profile matching step. Use an existing bool indicating that we are redoing this from existing metadata to always propagate any context size metadata in that case.
1 parent 1e0d289 commit b757d84

File tree

2 files changed

+26
-6
lines changed

2 files changed

+26
-6
lines changed

llvm/lib/Analysis/MemoryProfileInfo.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,13 @@ static MDNode *createMIBNode(LLVMContext &Ctx, ArrayRef<uint64_t> MIBCallStack,
241241
ColdBytes += TotalSize;
242242
// If we have the max cold context size from summary information and have
243243
// requested identification of contexts above a percentage of the max, see
244-
// if this context qualifies.
245-
if (MaxColdSize > 0 && MinPercentMaxColdSize < 100 &&
246-
TotalSize * 100 >= MaxColdSize * MinPercentMaxColdSize)
244+
// if this context qualifies. We should assume this is large if we rebuilt
245+
// the trie from existing metadata (i.e. to update after inlining), in
246+
// which case we don't have a MaxSize from the profile - we assume any
247+
// context size info in existence on the metadata should be propagated.
248+
if (BuiltFromExistingMetadata ||
249+
(MaxColdSize > 0 && MinPercentMaxColdSize < 100 &&
250+
TotalSize * 100 >= MaxColdSize * MinPercentMaxColdSize))
247251
LargeColdContext = true;
248252
}
249253
// Only add the context size info as metadata if we need it in the thin

llvm/unittests/Analysis/MemoryProfileInfoTest.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef)
638638
!0 = !{!1, !3, !5, !7, !9, !11}
639639
!1 = !{!2, !"cold"}
640640
!2 = !{i64 1, i64 2, i64 3}
641-
!3 = !{!4, !"cold"}
641+
!3 = !{!4, !"cold", !13}
642642
!4 = !{i64 1, i64 2, i64 4}
643643
!5 = !{!6, !"notcold"}
644644
!6 = !{i64 1, i64 5, i64 6}
@@ -648,6 +648,7 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef)
648648
!10 = !{i64 1, i64 8, i64 9}
649649
!11 = !{!12, !"hot"}
650650
!12 = !{i64 1, i64 8, i64 10}
651+
!13 = !{i64 123, i64 456}
651652
)IR");
652653

653654
Function *Func = M->getFunction("test");
@@ -683,10 +684,25 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef)
683684
auto *StackId = mdconst::dyn_extract<ConstantInt>(StackMD->getOperand(0));
684685
EXPECT_EQ(StackId->getZExtValue(), 1u);
685686
StackId = mdconst::dyn_extract<ConstantInt>(StackMD->getOperand(1));
686-
if (StackId->getZExtValue() == 2u)
687+
if (StackId->getZExtValue() == 2u) {
687688
EXPECT_EQ(getMIBAllocType(MIB), AllocationType::Cold);
688-
else if (StackId->getZExtValue() == 5u)
689+
// We should propagate the single context size info from the second cold
690+
// context above onto the new merged/trimmed context.
691+
ASSERT_EQ(MIB->getNumOperands(), 3u);
692+
MDNode *ContextSizePair = dyn_cast<MDNode>(MIB->getOperand(2));
693+
assert(ContextSizePair->getNumOperands() == 2);
694+
EXPECT_EQ(
695+
mdconst::dyn_extract<ConstantInt>(ContextSizePair->getOperand(0))
696+
->getZExtValue(),
697+
123u);
698+
EXPECT_EQ(
699+
mdconst::dyn_extract<ConstantInt>(ContextSizePair->getOperand(1))
700+
->getZExtValue(),
701+
456u);
702+
} else if (StackId->getZExtValue() == 5u) {
689703
EXPECT_EQ(getMIBAllocType(MIB), AllocationType::NotCold);
704+
ASSERT_EQ(MIB->getNumOperands(), 2u);
705+
}
690706
}
691707
}
692708

0 commit comments

Comments
 (0)