[llvm] [ThinLTO] Allow importing based on a workload definition (PR #74545)

Mircea Trofin via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 13 17:07:43 PST 2023


================
@@ -451,76 +451,79 @@ class WorkloadImportsManager : public ModuleImportsManager {
     auto &ValueInfos = SetIter->second;
     SmallVector<EdgeInfo, 128> GlobWorklist;
     for (auto &VI : llvm::make_early_inc_range(ValueInfos)) {
+      auto It = DefinedGVSummaries.find(VI.getGUID());
+      if (It != DefinedGVSummaries.end() &&
+          IsPrevailing(VI.getGUID(), It->second)) {
+        LLVM_DEBUG(
+            dbgs() << "[Workload] " << VI.name()
+                   << " has the prevailing variant already in the module "
+                   << ModName << ". No need to import\n");
+        continue;
+      }
       auto Candidates =
           qualifyCalleeCandidates(Index, VI.getSummaryList(), ModName);
 
       const GlobalValueSummary *GVS = nullptr;
-      FunctionImporter::ImportFailureReason LastReason =
-          FunctionImporter::ImportFailureReason::None;
-      for (const auto &Candidate : Candidates) {
-        /// We will prefer importing the prevailing candidate, if not, we'll
-        /// still pick the first available candidate. The reason we want to make
-        /// sure we do import the prevailing candidate is because the goal of
-        /// workload-awareness is to enable optimizations specializing the call
-        /// graph of that workload. Suppose a function is already defined in the
-        /// module, but it's not the prevailing variant. Suppose also we do not
-        /// inline it, but we could specialize it to the workload in other ways.
-        /// However, the linker would drop it in the favor of the prevailing
-        /// copy. Instead, by importing the prevailing variant (assuming also
-        /// the use of `-avail-extern-to-local`), we keep the specialization. We
-        /// could alteranatively make the non-prevailing variant local, but the
-        /// prevailing one is also the one for which we would have previously
-        /// collected profiles, making it preferrable.
-        LastReason = Candidate.first;
-        if (Candidate.first == FunctionImporter::ImportFailureReason::None) {
-          const bool Prevailing = IsPrevailing(VI.getGUID(), Candidate.second);
-          if (Prevailing || !GVS) {
-            if (!GVS && !Prevailing)
-              LLVM_DEBUG(dbgs()
-                         << "[Workload] Considering " << VI.name() << " from "
-                         << Candidate.second->modulePath() << " with linkage "
-                         << Candidate.second->linkage()
-                         << " although it's not prevailing, but it's the "
-                            "first available candidate.\n");
-            GVS = Candidate.second;
-            if (Prevailing) {
-              LLVM_DEBUG(dbgs()
-                         << "[Workload] Considering " << VI.name() << " from "
-                         << GVS->modulePath() << " with linkage "
-                         << GVS->linkage() << " because it's prevailing.\n");
-              break;
-            }
-          } else {
-            LLVM_DEBUG(dbgs() << "[Workload] Skipping " << VI.name() << " from "
-                              << Candidate.second->modulePath()
-                              << " with linkage " << Candidate.second->linkage()
-                              << " because it's not prevailing\n");
-          }
-        }
-      }
-      if (!GVS) {
+      auto PotentialCandidates = llvm::map_range(
+          llvm::make_filter_range(
----------------
mtrofin wrote:

I thought more about interposables. I think they could be imported if also made internal, wdyt? but separate patch. 

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


More information about the llvm-commits mailing list