Skip to content

Commit ea692d5

Browse files
teresajohnsonteresajohnson
committed
[MemProf] Make sure call clones without callsite node clones get updated
Because we may prune differing amounts of call context for different allocation contexts during matching (we only keep enough call context to distinguish cold from noncold paths), we can end up with different numbers of callsite node clones for different callsites in the same function. Any callsites that don't have node clones for all function clones should have their copies in those other function clones updated the same way as the version in the original function, which might be calling a clone of the callsite.
1 parent 2771d35 commit ea692d5

File tree

4 files changed

+371
-3
lines changed

4 files changed

+371
-3
lines changed

‎llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4553,6 +4553,34 @@ bool CallsiteContextGraph<DerivedCCG, FuncTy, CallTy>::assignFunctions() {
45534553
DenseMap<CallInfo, CallInfo> CallMap;
45544554
};
45554555

4556+
// Map to keep track of information needed to update calls in function clones
4557+
// when their corresponding callsite node was not itself cloned for that
4558+
// function clone. Because of call context pruning (i.e. we only keep as much
4559+
// caller information as needed to distinguish hot vs cold), we may not have
4560+
// caller edges coming to each callsite node from all possible function
4561+
// callers. A function clone may get created for other callsites in the
4562+
// function for which there are caller edges that were not pruned. Any other
4563+
// callsites in that function clone, which were not themselved cloned for
4564+
// that function clone, should get updated the same way as the corresponding
4565+
// callsite in the original function (which may call a clone of its callee).
4566+
//
4567+
// We build this map after completing function cloning for each function, so
4568+
// that we can record the information from its call maps before they are
4569+
// destructed. The map will be used as we update calls to update any still
4570+
// unassigned call clones. Note that we may create new node clones as we clone
4571+
// other functions, so later on we check which node clones were still not
4572+
// created. To this end, the inner map is a map from function clone number to
4573+
// the list of calls cloned for that function (can be more than one due to the
4574+
// Node's MatchingCalls array).
4575+
//
4576+
// The alternative is creating new callsite clone nodes below as we clone the
4577+
// function, but that is tricker to get right and likely more overhead.
4578+
//
4579+
// Inner map is a std::map so sorted by key (clone number), in order to get
4580+
// ordered remarks in the full LTO case.
4581+
DenseMap<const ContextNode *, std::map<unsigned, SmallVector<CallInfo, 0>>>
4582+
UnassignedCallClones;
4583+
45564584
// Walk all functions for which we saw calls with memprof metadata, and handle
45574585
// cloning for each of its calls.
45584586
for (auto &[Func, CallsWithMetadata] : FuncToCallsWithMetadata) {
@@ -4986,6 +5014,63 @@ bool CallsiteContextGraph<DerivedCCG, FuncTy, CallTy>::assignFunctions() {
49865014
}
49875015
}
49885016
}
5017+
5018+
if (FuncCloneInfos.size() < 2)
5019+
continue;
5020+
5021+
// In this case there is more than just the original function copy.
5022+
// Record call clones of any callsite nodes in the function that did not
5023+
// themselves get cloned for all of the function clones.
5024+
for (auto &Call : CallsWithMetadata) {
5025+
ContextNode *Node = getNodeForInst(Call);
5026+
if (!Node || !Node->hasCall() || Node->emptyContextIds())
5027+
continue;
5028+
// If Node has enough clones already to cover all function clones, we can
5029+
// skip it. Need to add one for the original copy.
5030+
// Use >= in case there were clones that were skipped due to having empty
5031+
// context ids
5032+
if (Node->Clones.size() + 1 >= FuncCloneInfos.size())
5033+
continue;
5034+
// First collect all function clones we cloned this callsite node for.
5035+
// They may not be sequential due to empty clones e.g.
5036+
DenseSet<unsigned> NodeCallClones;
5037+
for (auto *C : Node->Clones)
5038+
NodeCallClones.insert(C->Call.cloneNo());
5039+
unsigned I = 0;
5040+
// Now check all the function clones.
5041+
for (auto &FC : FuncCloneInfos) {
5042+
// Function clones should be sequential.
5043+
assert(FC.FuncClone.cloneNo() == I);
5044+
// Skip the first clone which got the original call.
5045+
// Also skip any other clones created for this Node.
5046+
if (++I == 1 || NodeCallClones.contains(I)) {
5047+
continue;
5048+
}
5049+
// Record the call clones created for this callsite in this function
5050+
// clone.
5051+
auto &CallVector = UnassignedCallClones[Node][I];
5052+
DenseMap<CallInfo, CallInfo> &CallMap = FC.CallMap;
5053+
CallInfo CallClone(Call);
5054+
if (auto It = CallMap.find(Call); It != CallMap.end())
5055+
CallClone = It->second;
5056+
else
5057+
// All but the original clone (skipped earlier) should have an entry
5058+
// for all calls.
5059+
assert(false && "Expected to find call in CallMap");
5060+
CallVector.push_back(CallClone);
5061+
// Need to do the same for all matching calls.
5062+
for (auto &MatchingCall : Node->MatchingCalls) {
5063+
CallInfo CallClone(MatchingCall);
5064+
if (auto It = CallMap.find(MatchingCall); It != CallMap.end())
5065+
CallClone = It->second;
5066+
else
5067+
// All but the original clone (skipped earlier) should have an entry
5068+
// for all calls.
5069+
assert(false && "Expected to find call in CallMap");
5070+
CallVector.push_back(CallClone);
5071+
}
5072+
}
5073+
}
49895074
}
49905075

49915076
uint8_t BothTypes =
@@ -5047,6 +5132,25 @@ bool CallsiteContextGraph<DerivedCCG, FuncTy, CallTy>::assignFunctions() {
50475132
// Update all the matching calls as well.
50485133
for (auto &Call : Node->MatchingCalls)
50495134
updateCall(Call, CalleeFunc);
5135+
5136+
// Now update all calls recorded earlier that are still in function clones
5137+
// which don't have a clone of this callsite node.
5138+
if (!UnassignedCallClones.contains(Node))
5139+
return;
5140+
DenseSet<unsigned> NodeCallClones;
5141+
for (auto *C : Node->Clones)
5142+
NodeCallClones.insert(C->Call.cloneNo());
5143+
auto &ClonedCalls = UnassignedCallClones[Node];
5144+
for (auto &[CloneNo, CallVector] : ClonedCalls) {
5145+
// Should start at 1 as we never create an entry for original node.
5146+
assert(CloneNo > 0);
5147+
// If we subsequently created a clone, skip this one.
5148+
if (NodeCallClones.contains(CloneNo))
5149+
continue;
5150+
// Use the original Node's CalleeFunc.
5151+
for (auto &Call : CallVector)
5152+
updateCall(Call, CalleeFunc);
5153+
}
50505154
};
50515155

50525156
// Performs DFS traversal starting from allocation nodes to update calls to

‎llvm/test/ThinLTO/X86/memprof-funcassigncloning2.ll‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
;; Similar to funcassigncloning.ll but hand modified to add another allocation
2+
;; whose pruned cold context only includes an immediate caller node that itself
3+
;; doesn't need cloning, but calls a cloned allocating function, and is in a
4+
;; function that gets cloned multiple times for a different callsite. This test
5+
;; makes sure the non-cloned callsite is correctly updated in all function
6+
;; clones. This case was missed because due to context pruning we don't have any
7+
;; caller edges for the first callsite, so the handling that kicks in to
8+
;; "reclone" other callsites in cloned functions was being missed.
9+
10+
; RUN: opt -thinlto-bc %s >%t.o
11+
; RUN: llvm-lto2 run %t.o -enable-memprof-context-disambiguation \
12+
; RUN: -supports-hot-cold-new \
13+
; RUN: -r=%t.o,main,plx \
14+
; RUN: -r=%t.o,_Znam, \
15+
; RUN: -memprof-verify-ccg -memprof-verify-nodes \
16+
; RUN: -pass-remarks=memprof-context-disambiguation -save-temps \
17+
; RUN: -o %t.out 2>&1 | FileCheck %s --check-prefix=REMARKS
18+
19+
; RUN: llvm-dis %t.out.1.4.opt.bc -o - | FileCheck %s --check-prefix=IR
20+
21+
22+
;; Try again but with distributed ThinLTO
23+
; RUN: llvm-lto2 run %t.o -enable-memprof-context-disambiguation \
24+
; RUN: -supports-hot-cold-new \
25+
; RUN: -thinlto-distributed-indexes \
26+
; RUN: -r=%t.o,main,plx \
27+
; RUN: -r=%t.o,_Znam, \
28+
; RUN: -memprof-verify-ccg -memprof-verify-nodes \
29+
; RUN: -pass-remarks=memprof-context-disambiguation \
30+
; RUN: -o %t2.out
31+
32+
;; Run ThinLTO backend
33+
; RUN: opt -passes=memprof-context-disambiguation \
34+
; RUN: -memprof-import-summary=%t.o.thinlto.bc \
35+
; RUN: -pass-remarks=memprof-context-disambiguation \
36+
; RUN: %t.o -S 2>&1 | FileCheck %s --check-prefix=IR \
37+
; RUN: --check-prefix=REMARKS
38+
39+
40+
source_filename = "funcassigncloning.ll"
41+
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
42+
target triple = "x86_64-unknown-linux-gnu"
43+
44+
;; Eventually this function will be cloned several times (for the calls to new
45+
;; for the various callers). However, function blah() includes an allocation
46+
;; whose cold context was trimmed above here. We therefore should assume that
47+
;; every caller of this function should call the same version of blah (which
48+
;; will be the cloned ".memprof.1" version.
49+
; Function Attrs: noinline optnone
50+
define internal void @_Z1EPPcS0_(ptr %buf1, ptr %buf2) #0 {
51+
entry:
52+
call void @blah(), !callsite !19
53+
%call = call ptr @_Znam(i64 noundef 10), !memprof !0, !callsite !7
54+
%call1 = call ptr @_Znam(i64 noundef 10), !memprof !8, !callsite !15
55+
ret void
56+
}
57+
58+
; REMARKS: call in clone _Z1EPPcS0_ assigned to call function clone blah.memprof.1
59+
; REMARKS: call in clone _Z1EPPcS0_.memprof.1 assigned to call function clone blah.memprof.1
60+
; REMARKS: call in clone _Z1EPPcS0_.memprof.2 assigned to call function clone blah.memprof.1
61+
; REMARKS: call in clone _Z1EPPcS0_.memprof.3 assigned to call function clone blah.memprof.1
62+
63+
; IR: define {{.*}} @_Z1EPPcS0_
64+
; IR: call {{.*}} @blah.memprof.1()
65+
; IR: define {{.*}} @_Z1EPPcS0_.memprof.2
66+
; IR: call {{.*}} @blah.memprof.1()
67+
; IR: define {{.*}} @_Z1EPPcS0_.memprof.3
68+
; IR: call {{.*}} @blah.memprof.1()
69+
70+
declare ptr @_Znam(i64)
71+
72+
define internal void @_Z1BPPcS0_() {
73+
entry:
74+
call void @_Z1EPPcS0_(ptr null, ptr null), !callsite !16
75+
ret void
76+
}
77+
78+
define internal void @_Z1CPPcS0_() {
79+
entry:
80+
call void @_Z1EPPcS0_(ptr null, ptr null), !callsite !17
81+
ret void
82+
}
83+
84+
define internal void @_Z1DPPcS0_() {
85+
entry:
86+
call void @_Z1EPPcS0_(ptr null, ptr null), !callsite !18
87+
ret void
88+
}
89+
90+
; Function Attrs: noinline optnone
91+
define i32 @main() #0 {
92+
entry:
93+
call void @_Z1BPPcS0_()
94+
call void @_Z1CPPcS0_()
95+
call void @_Z1DPPcS0_()
96+
ret i32 0
97+
}
98+
99+
define internal void @blah() #0 {
100+
entry:
101+
%call = call noalias noundef nonnull ptr @_Znam(i64 noundef 10) #6, !memprof !22, !callsite !21
102+
ret void
103+
}
104+
105+
define internal void @foo() #0 {
106+
entry:
107+
call void @blah(), !callsite !20
108+
ret void
109+
}
110+
111+
; uselistorder directives
112+
uselistorder ptr @_Znam, { 1, 0, 2 }
113+
114+
attributes #0 = { noinline optnone }
115+
116+
!0 = !{!1, !3, !5}
117+
!1 = !{!2, !"cold"}
118+
!2 = !{i64 -3461278137325233666, i64 -7799663586031895603}
119+
!3 = !{!4, !"notcold"}
120+
!4 = !{i64 -3461278137325233666, i64 -3483158674395044949}
121+
!5 = !{!6, !"notcold"}
122+
!6 = !{i64 -3461278137325233666, i64 -2441057035866683071}
123+
!7 = !{i64 -3461278137325233666}
124+
!8 = !{!9, !11, !13}
125+
!9 = !{!10, !"notcold"}
126+
!10 = !{i64 -1415475215210681400, i64 -2441057035866683071}
127+
!11 = !{!12, !"cold"}
128+
!12 = !{i64 -1415475215210681400, i64 -3483158674395044949}
129+
!13 = !{!14, !"notcold"}
130+
!14 = !{i64 -1415475215210681400, i64 -7799663586031895603}
131+
!15 = !{i64 -1415475215210681400}
132+
!16 = !{i64 -2441057035866683071}
133+
!17 = !{i64 -3483158674395044949}
134+
!18 = !{i64 -7799663586031895603}
135+
!19 = !{i64 123}
136+
!20 = !{i64 234}
137+
!21 = !{i64 345}
138+
!22 = !{!23, !25}
139+
!23 = !{!24, !"cold"}
140+
!24 = !{i64 345, i64 123}
141+
!25 = !{!26, !"notcold"}
142+
!26 = !{i64 345, i64 234}

‎llvm/test/Transforms/MemProfContextDisambiguation/funcassigncloning2.ll‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
;; Similar to funcassigncloning.ll but hand modified to add another allocation
2+
;; whose pruned cold context only includes an immediate caller node that itself
3+
;; doesn't need cloning, but calls a cloned allocating function, and is in a
4+
;; function that gets cloned multiple times for a different callsite. This test
5+
;; makes sure the non-cloned callsite is correctly updated in all function
6+
;; clones. This case was missed because due to context pruning we don't have any
7+
;; caller edges for the first callsite, so the handling that kicks in to
8+
;; "reclone" other callsites in cloned functions was being missed.
9+
10+
; RUN: opt -passes=memprof-context-disambiguation -supports-hot-cold-new \
11+
; RUN: -memprof-verify-ccg -memprof-verify-nodes \
12+
; RUN: -pass-remarks=memprof-context-disambiguation \
13+
; RUN: %s -S 2>&1 | FileCheck %s --check-prefix=IR --check-prefix=REMARKS
14+
15+
16+
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
17+
target triple = "x86_64-unknown-linux-gnu"
18+
19+
;; Eventually this function will be cloned several times (for the calls to new
20+
;; for the various callers). However, function blah() includes an allocation
21+
;; whose cold context was trimmed above here. We therefore should assume that
22+
;; every caller of this function should call the same version of blah (which
23+
;; will be the cloned ".memprof.1" version.
24+
define internal void @_Z1EPPcS0_(ptr %buf1, ptr %buf2) #0 {
25+
entry:
26+
call void @blah(), !callsite !19
27+
%call = call noalias noundef nonnull ptr @_Znam(i64 noundef 10) #6, !memprof !0, !callsite !7
28+
%call1 = call noalias noundef nonnull ptr @_Znam(i64 noundef 10) #6, !memprof !8, !callsite !15
29+
ret void
30+
}
31+
32+
; REMARKS: created clone blah.memprof.1
33+
; REMARKS: call in clone _Z1EPPcS0_ assigned to call function clone blah.memprof.1
34+
; REMARKS: call in clone _Z1EPPcS0_.memprof.1 assigned to call function clone blah.memprof.1
35+
; REMARKS: call in clone _Z1EPPcS0_.memprof.2 assigned to call function clone blah.memprof.1
36+
; REMARKS: call in clone _Z1EPPcS0_.memprof.3 assigned to call function clone blah.memprof.1
37+
38+
; IR: define {{.*}} @_Z1EPPcS0_
39+
; IR: call {{.*}} @blah.memprof.1()
40+
; IR: define {{.*}} @_Z1EPPcS0_.memprof.1
41+
; IR: call {{.*}} @blah.memprof.1()
42+
; IR: define {{.*}} @_Z1EPPcS0_.memprof.2
43+
; IR: call {{.*}} @blah.memprof.1()
44+
; IR: define {{.*}} @_Z1EPPcS0_.memprof.3
45+
; IR: call {{.*}} @blah.memprof.1()
46+
47+
declare ptr @_Znam(i64) #1
48+
49+
define internal void @_Z1BPPcS0_(ptr %0, ptr %1) {
50+
entry:
51+
call void @_Z1EPPcS0_(ptr noundef %0, ptr noundef %1), !callsite !16
52+
ret void
53+
}
54+
55+
; Function Attrs: noinline
56+
define internal void @_Z1CPPcS0_(ptr %0, ptr %1) #2 {
57+
entry:
58+
call void @_Z1EPPcS0_(ptr noundef %0, ptr noundef %1), !callsite !17
59+
ret void
60+
}
61+
62+
define internal void @_Z1DPPcS0_(ptr %0, ptr %1) #3 {
63+
entry:
64+
call void @_Z1EPPcS0_(ptr noundef %0, ptr noundef %1), !callsite !18
65+
ret void
66+
}
67+
68+
define internal void @blah() #0 {
69+
entry:
70+
%call = call noalias noundef nonnull ptr @_Znam(i64 noundef 10) #6, !memprof !22, !callsite !21
71+
ret void
72+
}
73+
74+
define internal void @foo() #0 {
75+
entry:
76+
call void @blah(), !callsite !20
77+
ret void
78+
}
79+
80+
; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: write)
81+
declare void @llvm.memset.p0.i64(ptr nocapture writeonly, i8, i64, i1 immarg) #4
82+
83+
declare i32 @sleep() #5
84+
85+
; uselistorder directives
86+
uselistorder ptr @_Znam, { 1, 0, 2 }
87+
88+
attributes #0 = { "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" }
89+
attributes #1 = { "no-trapping-math"="true" }
90+
attributes #2 = { noinline }
91+
attributes #3 = { "frame-pointer"="all" }
92+
attributes #4 = { nocallback nofree nounwind willreturn memory(argmem: write) }
93+
attributes #5 = { "disable-tail-calls"="true" }
94+
attributes #6 = { builtin }
95+
96+
!0 = !{!1, !3, !5}
97+
!1 = !{!2, !"cold"}
98+
!2 = !{i64 -3461278137325233666, i64 -7799663586031895603}
99+
!3 = !{!4, !"notcold"}
100+
!4 = !{i64 -3461278137325233666, i64 -3483158674395044949}
101+
!5 = !{!6, !"notcold"}
102+
!6 = !{i64 -3461278137325233666, i64 -2441057035866683071}
103+
!7 = !{i64 -3461278137325233666}
104+
!8 = !{!9, !11, !13}
105+
!9 = !{!10, !"notcold"}
106+
!10 = !{i64 -1415475215210681400, i64 -2441057035866683071}
107+
!11 = !{!12, !"cold"}
108+
!12 = !{i64 -1415475215210681400, i64 -3483158674395044949}
109+
!13 = !{!14, !"notcold"}
110+
!14 = !{i64 -1415475215210681400, i64 -7799663586031895603}
111+
!15 = !{i64 -1415475215210681400}
112+
!16 = !{i64 -2441057035866683071}
113+
!17 = !{i64 -3483158674395044949}
114+
!18 = !{i64 -7799663586031895603}
115+
!19 = !{i64 123}
116+
!20 = !{i64 234}
117+
!21 = !{i64 345}
118+
!22 = !{!23, !25}
119+
!23 = !{!24, !"cold"}
120+
!24 = !{i64 345, i64 123}
121+
!25 = !{!26, !"notcold"}
122+
!26 = !{i64 345, i64 234}

0 commit comments

Comments
 (0)