Skip to content

Commit d7d0e74

Browse files
[MemProf] Refactor single alloc type handling and use in more cases (#120290)
Emit message when we have aliased contexts that are conservatively hinted not cold. This is not a change in behavior, just in message when the -memprof-report-hinted-sizes flag is enabled.
1 parent 984cb79 commit d7d0e74

File tree

3 files changed

+34
-18
lines changed

3 files changed

+34
-18
lines changed

llvm/include/llvm/Analysis/MemoryProfileInfo.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,12 @@ class CallStackTrie {
117117
/// which is lower overhead and more direct than maintaining this metadata.
118118
/// Returns true if memprof metadata attached, false if not (attribute added).
119119
bool buildAndAttachMIBMetadata(CallBase *CI);
120+
121+
/// Add an attribute for the given allocation type to the call instruction.
122+
/// If hinted by reporting is enabled, a message is emitted with the given
123+
/// descriptor used to identify the category of single allocation type.
124+
void addSingleAllocTypeAttribute(CallBase *CI, AllocationType AT,
125+
StringRef Descriptor);
120126
};
121127

122128
/// Helper class to iterate through stack ids in both metadata (memprof MIB and

llvm/lib/Analysis/MemoryProfileInfo.cpp

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -278,26 +278,30 @@ bool CallStackTrie::buildMIBNodes(CallStackTrieNode *Node, LLVMContext &Ctx,
278278
return true;
279279
}
280280

281+
void CallStackTrie::addSingleAllocTypeAttribute(CallBase *CI, AllocationType AT,
282+
StringRef Descriptor) {
283+
addAllocTypeAttribute(CI->getContext(), CI, AT);
284+
if (MemProfReportHintedSizes) {
285+
std::vector<ContextTotalSize> ContextSizeInfo;
286+
collectContextSizeInfo(Alloc, ContextSizeInfo);
287+
for (const auto &[FullStackId, TotalSize] : ContextSizeInfo) {
288+
errs() << "MemProf hinting: Total size for full allocation context hash "
289+
<< FullStackId << " and " << Descriptor << " alloc type "
290+
<< getAllocTypeAttributeString(AT) << ": " << TotalSize << "\n";
291+
}
292+
}
293+
}
294+
281295
// Build and attach the minimal necessary MIB metadata. If the alloc has a
282296
// single allocation type, add a function attribute instead. Returns true if
283297
// memprof metadata attached, false if not (attribute added).
284298
bool CallStackTrie::buildAndAttachMIBMetadata(CallBase *CI) {
285-
auto &Ctx = CI->getContext();
286299
if (hasSingleAllocType(Alloc->AllocTypes)) {
287-
addAllocTypeAttribute(Ctx, CI, (AllocationType)Alloc->AllocTypes);
288-
if (MemProfReportHintedSizes) {
289-
std::vector<ContextTotalSize> ContextSizeInfo;
290-
collectContextSizeInfo(Alloc, ContextSizeInfo);
291-
for (const auto &[FullStackId, TotalSize] : ContextSizeInfo) {
292-
errs()
293-
<< "MemProf hinting: Total size for full allocation context hash "
294-
<< FullStackId << " and single alloc type "
295-
<< getAllocTypeAttributeString((AllocationType)Alloc->AllocTypes)
296-
<< ": " << TotalSize << "\n";
297-
}
298-
}
300+
addSingleAllocTypeAttribute(CI, (AllocationType)Alloc->AllocTypes,
301+
"single");
299302
return false;
300303
}
304+
auto &Ctx = CI->getContext();
301305
std::vector<uint64_t> MIBCallStack;
302306
MIBCallStack.push_back(AllocStackId);
303307
std::vector<Metadata *> MIBNodes;
@@ -314,8 +318,9 @@ bool CallStackTrie::buildAndAttachMIBMetadata(CallBase *CI) {
314318
// If there exists corner case that CallStackTrie has one chain to leaf
315319
// and all node in the chain have multi alloc type, conservatively give
316320
// it non-cold allocation type.
317-
// FIXME: Avoid this case before memory profile created.
318-
addAllocTypeAttribute(Ctx, CI, AllocationType::NotCold);
321+
// FIXME: Avoid this case before memory profile created. Alternatively, select
322+
// hint based on fraction cold.
323+
addSingleAllocTypeAttribute(CI, AllocationType::NotCold, "indistinguishable");
319324
return false;
320325
}
321326

llvm/test/Transforms/PGOProfile/memprof_loop_unroll.ll

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
;; Tests memprof when contains loop unroll.
1+
;; Tests memprof when contains loop unroll of allocation, where the unrolled
2+
;; allocations have the same context but different allocation types.
23

34
;; Avoid failures on big-endian systems that can't read the profile properly
45
; REQUIRES: x86_64-linux
@@ -9,8 +10,12 @@
910
;; $ clang++ -gmlt -fdebug-info-for-profiling -S %S/Inputs/memprof_loop_unroll_b.cc -emit-llvm
1011

1112
; RUN: llvm-profdata merge %S/Inputs/memprof_loop_unroll.memprofraw --profiled-binary %S/Inputs/memprof_loop_unroll.exe -o %t.memprofdata
12-
; RUN: opt < %s -passes='memprof-use<profile-filename=%t.memprofdata>' -S | FileCheck %s
13+
; RUN: opt < %s -passes='memprof-use<profile-filename=%t.memprofdata>' -S -memprof-report-hinted-sizes 2>&1 | FileCheck %s
1314

15+
;; Conservatively annotate as not cold. We get two messages as there are two
16+
;; unrolled copies of the allocation.
17+
; CHECK: MemProf hinting: Total size for full allocation context hash {{.*}} and indistinguishable alloc type notcold: 4
18+
; CHECK: MemProf hinting: Total size for full allocation context hash {{.*}} and indistinguishable alloc type notcold: 4
1419
; CHECK: call {{.*}} @_Znam{{.*}} #[[ATTR:[0-9]+]]
1520
; CHECK: attributes #[[ATTR]] = { builtin allocsize(0) "memprof"="notcold" }
1621
; CHECK-NOT: stackIds: ()
@@ -93,4 +98,4 @@ attributes #2 = { builtin allocsize(0) }
9398
!27 = distinct !{!27, !28, !23, !29}
9499
!28 = !DILocation(line: 5, column: 5, scope: !10)
95100
!29 = !{!"llvm.loop.mustprogress"}
96-
!30 = !DILocation(line: 8, column: 1, scope: !10)
101+
!30 = !DILocation(line: 8, column: 1, scope: !10)

0 commit comments

Comments
 (0)