[llvm] [FMV][GlobalOpt] Add an option for static resolution of non-FMV callers. (PR #123757)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Alexandros Lamprineas (labrinea)

<details>
<summary>Changes</summary>

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 https://github.com/llvm/llvm-project/pull/123383.

---
Full diff: https://github.com/llvm/llvm-project/pull/123757.diff


1 Files Affected:

- (modified) llvm/lib/Transforms/IPO/GlobalOpt.cpp (+24-11) 


``````````diff
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index eb97d8b4a74f35..00c20ad5f37094 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -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 "
@@ -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);
@@ -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() ||

``````````

</details>


https://github.com/llvm/llvm-project/pull/123757