Skip to content

[ctxprof][nfc] Make computeImportForFunction a member of ModuleImportsManager #134011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Apr 2, 2025

No description provided.

Copy link
Member Author

mtrofin commented Apr 2, 2025

@mtrofin mtrofin marked this pull request as ready for review April 2, 2025 15:43
@llvmbot llvmbot added LTO Link time optimization (regular/full LTO or ThinLTO) llvm:transforms labels Apr 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 2, 2025

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-lto

Author: Mircea Trofin (mtrofin)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/FunctionImport.cpp (+14-12)
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index faa052bb4d5b6..ae3b45a11996e 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -497,6 +497,13 @@ static const char *getFailureName(FunctionImporter::ImportFailureReason Reason);
 
 /// Determine the list of imports and exports for each module.
 class ModuleImportsManager {
+  void computeImportForFunction(
+      const FunctionSummary &Summary, unsigned Threshold,
+      const GVSummaryMapTy &DefinedGVSummaries,
+      SmallVectorImpl<EdgeInfo> &Worklist, GlobalsImporter &GVImporter,
+      FunctionImporter::ImportMapTy &ImportList,
+      FunctionImporter::ImportThresholdsTy &ImportThresholds);
+
 protected:
   function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
       IsPrevailing;
@@ -851,14 +858,11 @@ getFailureName(FunctionImporter::ImportFailureReason Reason) {
 /// Compute the list of functions to import for a given caller. Mark these
 /// imported functions and the symbols they reference in their source module as
 /// exported from their source module.
-static void computeImportForFunction(
-    const FunctionSummary &Summary, const ModuleSummaryIndex &Index,
-    const unsigned Threshold, const GVSummaryMapTy &DefinedGVSummaries,
-    function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
-        isPrevailing,
+void ModuleImportsManager::computeImportForFunction(
+    const FunctionSummary &Summary, const unsigned Threshold,
+    const GVSummaryMapTy &DefinedGVSummaries,
     SmallVectorImpl<EdgeInfo> &Worklist, GlobalsImporter &GVImporter,
     FunctionImporter::ImportMapTy &ImportList,
-    DenseMap<StringRef, FunctionImporter::ExportSetTy> *ExportLists,
     FunctionImporter::ImportThresholdsTy &ImportThresholds) {
   GVImporter.onImportingSummary(Summary);
   static int ImportCount = 0;
@@ -1063,9 +1067,8 @@ void ModuleImportsManager::computeImportForModule(
       // Skip import for global variables
       continue;
     LLVM_DEBUG(dbgs() << "Initialize import for " << VI << "\n");
-    computeImportForFunction(*FuncSummary, Index, ImportInstrLimit,
-                             DefinedGVSummaries, IsPrevailing, Worklist, GVI,
-                             ImportList, ExportLists, ImportThresholds);
+    computeImportForFunction(*FuncSummary, ImportInstrLimit, DefinedGVSummaries,
+                             Worklist, GVI, ImportList, ImportThresholds);
   }
 
   // Process the newly imported functions and add callees to the worklist.
@@ -1075,9 +1078,8 @@ void ModuleImportsManager::computeImportForModule(
     auto Threshold = std::get<1>(GVInfo);
 
     if (auto *FS = dyn_cast<FunctionSummary>(Summary))
-      computeImportForFunction(*FS, Index, Threshold, DefinedGVSummaries,
-                               IsPrevailing, Worklist, GVI, ImportList,
-                               ExportLists, ImportThresholds);
+      computeImportForFunction(*FS, Threshold, DefinedGVSummaries, Worklist,
+                               GVI, ImportList, ImportThresholds);
   }
 
   // Print stats about functions considered but rejected for importing

@mtrofin mtrofin force-pushed the users/mtrofin/04-01-_ctxprof_nfc_make_computeimportforfunction_a_member_of_moduleimportsmanager_ branch from 68b499f to 1081208 Compare April 2, 2025 19:03
@mtrofin mtrofin force-pushed the users/mtrofin/04-01-_ctxprof_option_to_move_a_whole_tree_to_its_own_module branch from 96bf973 to 1f5b366 Compare April 2, 2025 19:03
Copy link
Contributor

@kazutakahirata kazutakahirata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Member Author

mtrofin commented Apr 3, 2025

Merge activity

  • Apr 2, 9:11 PM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Apr 2, 9:16 PM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 2, 9:18 PM EDT: A user merged this pull request with Graphite.

@mtrofin mtrofin force-pushed the users/mtrofin/04-01-_ctxprof_option_to_move_a_whole_tree_to_its_own_module branch from 1f5b366 to 1b0c0a4 Compare April 3, 2025 01:13
Base automatically changed from users/mtrofin/04-01-_ctxprof_option_to_move_a_whole_tree_to_its_own_module to main April 3, 2025 01:15
@mtrofin mtrofin force-pushed the users/mtrofin/04-01-_ctxprof_nfc_make_computeimportforfunction_a_member_of_moduleimportsmanager_ branch from 1081208 to 7a534bf Compare April 3, 2025 01:16
@mtrofin mtrofin merged commit d59b2c4 into main Apr 3, 2025
6 of 10 checks passed
@mtrofin mtrofin deleted the users/mtrofin/04-01-_ctxprof_nfc_make_computeimportforfunction_a_member_of_moduleimportsmanager_ branch April 3, 2025 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:transforms LTO Link time optimization (regular/full LTO or ThinLTO)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants