Skip to content

Commit

Permalink
Automerge: [FMV][GlobalOpt] Add an option for static resolution of no…
Browse files Browse the repository at this point in the history
…n-FMV callers. (#123757)

Adds `optimize-non-fmv-callers` (disabled by default) as a short term
workaround to keep the llvm testsuite bots green, until we decide what
is the right solution for the problem which was previously addressed
with llvm/llvm-project#123383.
  • Loading branch information
labrinea authored and github-actions[bot] committed Jan 21, 2025
2 parents 63d858c + cf6d79a commit feef478
Showing 1 changed file with 24 additions and 11 deletions.
35 changes: 24 additions & 11 deletions llvm/lib/Transforms/IPO/GlobalOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,21 @@ STATISTIC(NumIFuncsDeleted, "Number of IFuncs removed");
STATISTIC(NumGlobalArraysPadded,
"Number of global arrays padded to alignment boundary");

// FIXME:
// Optimizing non-FMV callers is causing a regression in the llvm test suite,
// specifically a 'predres' version is unexpectedly trapping on GravitonG4.
// My explanation is that when the caller in not a versioned function, the
// compiler exclusively relies on the command line option, or target attribute
// to deduce whether a feature is available. However, there is no guarantee
// that in reality the host supports those implied features, which arguably
// is a user error. This option allows disabling the optimization as a short
// term workaround to keep the bots green.
static cl::opt<bool>
OptimizeNonFMVCallers("optimize-non-fmv-callers",
cl::desc("Statically resolve calls to versioned "
"functions from non-versioned callers."),
cl::init(false), cl::Hidden);

static cl::opt<bool>
EnableColdCCStressTest("enable-coldcc-stress-test",
cl::desc("Enable stress test of coldcc by adding "
Expand Down Expand Up @@ -2715,6 +2730,9 @@ static bool OptimizeNonTrivialIFuncs(

assert(!Callees.empty() && "Expecting successful collection of versions");

LLVM_DEBUG(dbgs() << "Statically resolving calls to function "
<< Resolver->getName() << "\n");

// Cache the feature mask for each callee.
for (Function *Callee : Callees) {
auto [It, Inserted] = FeatureMask.try_emplace(Callee);
Expand Down Expand Up @@ -2785,20 +2803,15 @@ static bool OptimizeNonTrivialIFuncs(
} else {
// We can't reason much about non-FMV callers. Just pick the highest
// priority callee if it matches, otherwise bail.
// if (I > 0 || !implies(CallerBits, CalleeBits))
//
// FIXME: This is causing a regression in the llvm test suite,
// specifically a 'predres' version is unexpectedly trapping on
// GravitonG4. My explanation is that when the caller in not a
// versioned function, the compiler exclusively relies on the
// command line option, or target attribute to deduce whether a
// feature is available. However, there is no guarantee that in
// reality the host supports those implied features.
continue;
if (!OptimizeNonFMVCallers || I > 0 || !implies(CallerBits, CalleeBits))
continue;
}
auto &Calls = CallSites[Caller];
for (CallBase *CS : Calls)
for (CallBase *CS : Calls) {
LLVM_DEBUG(dbgs() << "Redirecting call " << Caller->getName() << " -> "
<< Callee->getName() << "\n");
CS->setCalledOperand(Callee);
}
Changed = true;
}
if (IF.use_empty() ||
Expand Down

0 comments on commit feef478

Please sign in to comment.