Skip to content

Commit

Permalink
[GlobalOpt] Cache whether CC is changeable (#71381)
Browse files Browse the repository at this point in the history
The hasAddressTaken() call in hasOnlyColdCalls() has quadratic
complexity if there are many cold calls to a function: We're going to
visit each call of the function, and then for each of them iterate all
the users of the function.

We've recently encountered a case where GlobalOpt spends more than an
hour in these hasAddressTaken() checks when full LTO is used.

Avoid this by moving the hasAddressTaken() check into hasChangeableCC()
and caching its result, so it is only computed once per function.

(cherry picked from commit e360a16)
  • Loading branch information
nikic authored and tru committed Nov 14, 2023
1 parent 0a12742 commit 12c6ee8
Showing 1 changed file with 22 additions and 8 deletions.
30 changes: 22 additions & 8 deletions llvm/lib/Transforms/IPO/GlobalOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1701,13 +1701,16 @@ static void RemoveAttribute(Function *F, Attribute::AttrKind A) {
/// idea here is that we don't want to mess with the convention if the user
/// explicitly requested something with performance implications like coldcc,
/// GHC, or anyregcc.
static bool hasChangeableCC(Function *F) {
static bool hasChangeableCCImpl(Function *F) {
CallingConv::ID CC = F->getCallingConv();

// FIXME: Is it worth transforming x86_stdcallcc and x86_fastcallcc?
if (CC != CallingConv::C && CC != CallingConv::X86_ThisCall)
return false;

if (F->isVarArg())
return false;

// FIXME: Change CC for the whole chain of musttail calls when possible.
//
// Can't change CC of the function that either has musttail calls, or is a
Expand All @@ -1727,7 +1730,16 @@ static bool hasChangeableCC(Function *F) {
if (BB.getTerminatingMustTailCall())
return false;

return true;
return !F->hasAddressTaken();
}

using ChangeableCCCacheTy = SmallDenseMap<Function *, bool, 8>;
static bool hasChangeableCC(Function *F,
ChangeableCCCacheTy &ChangeableCCCache) {
auto Res = ChangeableCCCache.try_emplace(F, false);
if (Res.second)
Res.first->second = hasChangeableCCImpl(F);
return Res.first->second;
}

/// Return true if the block containing the call site has a BlockFrequency of
Expand Down Expand Up @@ -1781,7 +1793,8 @@ static void changeCallSitesToColdCC(Function *F) {
// coldcc calling convention.
static bool
hasOnlyColdCalls(Function &F,
function_ref<BlockFrequencyInfo &(Function &)> GetBFI) {
function_ref<BlockFrequencyInfo &(Function &)> GetBFI,
ChangeableCCCacheTy &ChangeableCCCache) {
for (BasicBlock &BB : F) {
for (Instruction &I : BB) {
if (CallInst *CI = dyn_cast<CallInst>(&I)) {
Expand All @@ -1800,8 +1813,7 @@ hasOnlyColdCalls(Function &F,
if (!CalledFn->hasLocalLinkage())
return false;
// Check if it's valid to use coldcc calling convention.
if (!hasChangeableCC(CalledFn) || CalledFn->isVarArg() ||
CalledFn->hasAddressTaken())
if (!hasChangeableCC(CalledFn, ChangeableCCCache))
return false;
BlockFrequencyInfo &CallerBFI = GetBFI(F);
if (!isColdCallSite(*CI, CallerBFI))
Expand Down Expand Up @@ -1931,9 +1943,10 @@ OptimizeFunctions(Module &M,

bool Changed = false;

ChangeableCCCacheTy ChangeableCCCache;
std::vector<Function *> AllCallsCold;
for (Function &F : llvm::make_early_inc_range(M))
if (hasOnlyColdCalls(F, GetBFI))
if (hasOnlyColdCalls(F, GetBFI, ChangeableCCCache))
AllCallsCold.push_back(&F);

// Optimize functions.
Expand Down Expand Up @@ -1995,7 +2008,7 @@ OptimizeFunctions(Module &M,
continue;
}

if (hasChangeableCC(&F) && !F.isVarArg() && !F.hasAddressTaken()) {
if (hasChangeableCC(&F, ChangeableCCCache)) {
NumInternalFunc++;
TargetTransformInfo &TTI = GetTTI(F);
// Change the calling convention to coldcc if either stress testing is
Expand All @@ -2005,14 +2018,15 @@ OptimizeFunctions(Module &M,
if (EnableColdCCStressTest ||
(TTI.useColdCCForColdCall(F) &&
isValidCandidateForColdCC(F, GetBFI, AllCallsCold))) {
ChangeableCCCache.erase(&F);
F.setCallingConv(CallingConv::Cold);
changeCallSitesToColdCC(&F);
Changed = true;
NumColdCC++;
}
}

if (hasChangeableCC(&F) && !F.isVarArg() && !F.hasAddressTaken()) {
if (hasChangeableCC(&F, ChangeableCCCache)) {
// If this function has a calling convention worth changing, is not a
// varargs function, and is only called directly, promote it to use the
// Fast calling convention.
Expand Down

0 comments on commit 12c6ee8

Please sign in to comment.