[nfc][thinlto] Separate `selectCallee` legality from cutoffs

[mtrofin]

Summary

This makes it easier to reuse the legality part for other import
policies that wouldn't use thresholds.

Importing un-inlinable functions is also legal, because they could be
further specialized in a context-specific way, without inlining.

[tejohnson]

lgtm

llvm/lib/Transforms/IPO/FunctionImport.cpp

static std::unique_ptr<Module> loadFile(const std::string &FileName,
   if (!Result) {
     Err.print("function-import", errs());
     report_fatal_error("Abort");
   }
 
   return Result;
 }
 
-/// Given a list of possible callee implementation for a call site, select one
-/// that fits the \p Threshold.
-///
-/// FIXME: select "best" instead of first that fits. But what is "best"?
-/// - The smallest: more likely to be inlined.
-/// - The one with the least outgoing edges (already well optimized).
-/// - One from a module already being imported from in order to reduce the
-///   number of source modules parsed/linked.
-/// - One that has PGO data attached.
-/// - [insert you fancy metric here]
-static const GlobalValueSummary *
-selectCallee(const ModuleSummaryIndex &Index,
-             ArrayRef<std::unique_ptr<GlobalValueSummary>> CalleeSummaryList,
-             unsigned Threshold, StringRef CallerModulePath,
-             FunctionImporter::ImportFailureReason &Reason,
-             GlobalValue::GUID GUID) {
-  Reason = FunctionImporter::ImportFailureReason::None;
-  auto It = llvm::find_if(
+/// Given a list of possible callee implementation for a call site, qualify the
+/// legality of importing each. The return is a range of pairs. Each pair
+/// corresponds to a candidate. The first value is the ImportFailureReason for
+/// that candidate, the second is the candidate.
+static auto qualifyCalleeCandidates(
+    const ModuleSummaryIndex &Index,
+    StringRef CallerModulePath) {
+  return llvm::map_range(
       CalleeSummaryList,
-      [&](const std::unique_ptr<GlobalValueSummary> &SummaryPtr) {
+      [&Index, CalleeSummaryList,
+       CallerModulePath](const std::unique_ptr<GlobalValueSummary> &SummaryPtr)
+          -> std::pair<FunctionImporter::ImportFailureReason,
+                       const GlobalValueSummary *> {
         auto *GVSummary = SummaryPtr.get();
-        if (!Index.isGlobalValueLive(GVSummary)) {
-          Reason = FunctionImporter::ImportFailureReason::NotLive;
-          return false;
-        }
+        if (!Index.isGlobalValueLive(GVSummary))
+          return {FunctionImporter::ImportFailureReason::NotLive, GVSummary};
 
-        if (GlobalValue::isInterposableLinkage(GVSummary->linkage())) {
-          Reason = FunctionImporter::ImportFailureReason::InterposableLinkage;
-          // There is no point in importing these, we can't inline them
-          return false;
-        }
+        if (GlobalValue::isInterposableLinkage(GVSummary->linkage()))
+          return {FunctionImporter::ImportFailureReason::InterposableLinkage,
+                  GVSummary};
 
         auto *Summary = cast<FunctionSummary>(GVSummary->getBaseObject());
 
         // If this is a local function, make sure we import the copy
         // in the caller's module. The only time a local function can
         // share an entry in the index is if there is a local with the same name
         // in another module that had the same source file name (in a different
         // directory), where each was compiled in their own directory so there
         // was not distinguishing path.
         // However, do the import from another module if there is only one
         // entry in the list - in that case this must be a reference due
         // to indirect call profile data, since a function pointer can point to
         // a local in another module.
         if (GlobalValue::isLocalLinkage(Summary->linkage()) &&
             CalleeSummaryList.size() > 1 &&
-            Summary->modulePath() != CallerModulePath) {
-          Reason =
-              FunctionImporter::ImportFailureReason::LocalLinkageNotInModule;
-          return false;
-        }
-
-        if ((Summary->instCount() > Threshold) &&
-            !Summary->fflags().AlwaysInline && !ForceImportAll) {
-          Reason = FunctionImporter::ImportFailureReason::TooLarge;
-          return false;
-        }
+            Summary->modulePath() != CallerModulePath)
+          return {
+              FunctionImporter::ImportFailureReason::LocalLinkageNotInModule,
+              GVSummary};
 
         // Skip if it isn't legal to import (e.g. may reference unpromotable
         // locals).
-        if (Summary->notEligibleToImport()) {
-          Reason = FunctionImporter::ImportFailureReason::NotEligible;
-          return false;
-        }
+        if (Summary->notEligibleToImport())
+          return {FunctionImporter::ImportFailureReason::NotEligible,
+                  GVSummary};
+
+        return {FunctionImporter::ImportFailureReason::None, GVSummary};
+      });
+}
+
+/// Given a list of possible callee implementation for a call site, select one
+/// that fits the \p Threshold. If none are found, the Reason will give the last
+/// reason for the failure (last, in the order of CalleeSummaryList entries).
+///
+/// FIXME: select "best" instead of first that fits. But what is "best"?
+/// - The smallest: more likely to be inlined.
+/// - The one with the least outgoing edges (already well optimized).
+/// - One from a module already being imported from in order to reduce the
+///   number of source modules parsed/linked.
+/// - One that has PGO data attached.
+/// - [insert you fancy metric here]
+static const GlobalValueSummary *
+selectCallee(const ModuleSummaryIndex &Index,
+             ArrayRef<std::unique_ptr<GlobalValueSummary>> CalleeSummaryList,
+             unsigned Threshold, StringRef CallerModulePath,
+             FunctionImporter::ImportFailureReason &Reason) {
+  auto QualifiedCandidates =
+      qualifyCalleeCandidates(Index, CalleeSummaryList, CallerModulePath);
+  for (auto QualifiedValue : QualifiedCandidates) {
+    Reason = QualifiedValue.first;
+    if (Reason != FunctionImporter::ImportFailureReason::None)
+      continue;
+    auto *Summary =
+        cast<FunctionSummary>(QualifiedValue.second->getBaseObject());
+
+    if ((Summary->instCount() > Threshold) && !Summary->fflags().AlwaysInline &&
+        !ForceImportAll) {
+      Reason = FunctionImporter::ImportFailureReason::TooLarge;
+      continue;
 
-        // Don't bother importing if we can't inline it anyway.
-        if (Summary->fflags().NoInline && !ForceImportAll) {
-          Reason = FunctionImporter::ImportFailureReason::NoInline;
-          return false;
-        }
+      continue;
 
-        return true;
-      });
-  if (It == CalleeSummaryList.end())
-    return nullptr;
-
-  return cast<GlobalValueSummary>(It->get());
+    return Summary;
+  }
 }
 
 namespace {
 
 using EdgeInfo =
     std::tuple<const GlobalValueSummary *, unsigned /* Threshold */>;
 
 } // anonymous namespace
if (CalleeSummary) {
                  "Expected FailureInfo for previously rejected candidate");
           FailureInfo->Attempts++;
         }
         continue;
       }
 
       FunctionImporter::ImportFailureReason Reason;
       CalleeSummary = selectCallee(Index, VI.getSummaryList(), NewThreshold,
-                                   Summary.modulePath(), Reason, VI.getGUID());
+                                   Summary.modulePath(), Reason);
       if (!CalleeSummary) {
         // Update with new larger threshold if this was a retry (otherwise
         // we would have already inserted with NewThreshold above). Also
         // update failure info if requested.
         if (PreviouslyVisited) {
           ProcessedThreshold = NewThreshold;
           if (PrintImportFailures) {
             assert(FailureInfo &&