Skip to content

Commit ca562ba

Browse files
committed
Address comments
1 parent f6ec3fa commit ca562ba

File tree

1 file changed

+13
-6
lines changed

1 file changed

+13
-6
lines changed

llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2070,7 +2070,9 @@ static unsigned getMemProfCloneNum(const Function &F) {
20702070
auto Pos = F.getName().find_last_of('.');
20712071
assert(Pos > 0);
20722072
unsigned CloneNo;
2073-
F.getName().drop_front(Pos + 1).getAsInteger(10, CloneNo);
2073+
auto Error = F.getName().drop_front(Pos + 1).getAsInteger(10, CloneNo);
2074+
assert(!Error);
2075+
(void)Error;
20742076
return CloneNo;
20752077
}
20762078

@@ -4025,7 +4027,7 @@ void IndexCallsiteContextGraph::updateCall(CallInfo &CallerCall,
40254027
"Caller cannot be an allocation which should not have profiled calls");
40264028
assert(CI->Clones.size() > CallerCall.cloneNo());
40274029
auto NewCalleeCloneNo = CalleeFunc.cloneNo();
4028-
auto CurCalleeCloneNo = CI->Clones[CallerCall.cloneNo()];
4030+
auto &CurCalleeCloneNo = CI->Clones[CallerCall.cloneNo()];
40294031
// If we already assigned this callsite to call a specific non-default
40304032
// clone (i.e. not the original function which is clone 0), ensure that we
40314033
// aren't trying to now update it to call a different clone, which is
@@ -4036,7 +4038,7 @@ void IndexCallsiteContextGraph::updateCall(CallInfo &CallerCall,
40364038
<< "\n");
40374039
MismatchedCloneAssignments++;
40384040
}
4039-
CI->Clones[CallerCall.cloneNo()] = NewCalleeCloneNo;
4041+
CurCalleeCloneNo = NewCalleeCloneNo;
40404042
}
40414043

40424044
// Update the debug information attached to NewFunc to use the clone Name. Note
@@ -4753,7 +4755,8 @@ bool CallsiteContextGraph<DerivedCCG, FuncTy, CallTy>::assignFunctions() {
47534755
if (!FuncCloneToCurNodeCloneMap.count(CF.first))
47544756
return CF.first;
47554757
}
4756-
assert(false);
4758+
assert(false &&
4759+
"Expected an available func clone for this callsite clone");
47574760
};
47584761

47594762
// See if we can use existing function clone. Walk through
@@ -4900,12 +4903,16 @@ bool CallsiteContextGraph<DerivedCCG, FuncTy, CallTy>::assignFunctions() {
49004903
// easily distinguish between callsites explicitly assigned to clone 0
49014904
// vs those never assigned, which can lead to multiple updates of the
49024905
// calls when invoking updateCall below, with mismatched clone values.
4906+
// TODO: Add a flag to the callsite nodes or some other mechanism to
4907+
// better distinguish and identify callsite clones that are not getting
4908+
// assigned to function clones as expected.
49034909
if (!FuncCloneAssignedToCurCallsiteClone) {
49044910
FuncCloneAssignedToCurCallsiteClone = FindFirstAvailFuncClone();
4905-
assert(FuncCloneAssignedToCurCallsiteClone);
4911+
assert(FuncCloneAssignedToCurCallsiteClone &&
4912+
"No available func clone for this callsite clone");
49064913
AssignCallsiteCloneToFuncClone(
49074914
FuncCloneAssignedToCurCallsiteClone, Call, Clone,
4908-
AllocationCallToContextNodeMap.count(Call));
4915+
/*IsAlloc=*/AllocationCallToContextNodeMap.contains(Call));
49094916
}
49104917
}
49114918
if (VerifyCCG) {

0 commit comments

Comments
 (0)