Skip to content

[nfc][ctx_prof] Use one flag for the "use" scenario #103377

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

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Aug 13, 2024

No need to have two flags, one for the thinlink and one for compilation.

No need to have two flags, one for the thinlink and one for compilation.
@llvmbot llvmbot added LTO Link time optimization (regular/full LTO or ThinLTO) llvm:transforms labels Aug 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 13, 2024

@llvm/pr-subscribers-lto

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

No need to have two flags, one for the thinlink and one for compilation.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/FunctionImport.cpp (+5-7)
  • (modified) llvm/test/ThinLTO/X86/ctxprof.ll (+1-1)
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 3364628be841e3..6ae89a49b6b9a3 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -174,9 +174,7 @@ static cl::opt<std::string> WorkloadDefinitions(
              "}"),
     cl::Hidden);
 
-static cl::opt<std::string>
-    ContextualProfile("thinlto-pgo-ctx-prof",
-                      cl::desc("Path to a contextual profile."), cl::Hidden);
+extern cl::opt<std::string> UseCtxProfile;
 
 namespace llvm {
 extern cl::opt<bool> EnableMemProfContextDisambiguation;
@@ -671,7 +669,7 @@ class WorkloadImportsManager : public ModuleImportsManager {
 
   void loadFromCtxProf() {
     std::error_code EC;
-    auto BufferOrErr = MemoryBuffer::getFileOrSTDIN(ContextualProfile);
+    auto BufferOrErr = MemoryBuffer::getFileOrSTDIN(UseCtxProfile);
     if (std::error_code EC = BufferOrErr.getError()) {
       report_fatal_error("Failed to open contextual profile file");
       return;
@@ -722,12 +720,12 @@ class WorkloadImportsManager : public ModuleImportsManager {
       const ModuleSummaryIndex &Index,
       DenseMap<StringRef, FunctionImporter::ExportSetTy> *ExportLists)
       : ModuleImportsManager(IsPrevailing, Index, ExportLists) {
-    if (ContextualProfile.empty() == WorkloadDefinitions.empty()) {
+    if (UseCtxProfile.empty() == WorkloadDefinitions.empty()) {
       report_fatal_error(
           "Pass only one of: -thinlto-pgo-ctx-prof or -thinlto-workload-def");
       return;
     }
-    if (!ContextualProfile.empty())
+    if (!UseCtxProfile.empty())
       loadFromCtxProf();
     else
       loadFromJson();
@@ -749,7 +747,7 @@ std::unique_ptr<ModuleImportsManager> ModuleImportsManager::create(
         IsPrevailing,
     const ModuleSummaryIndex &Index,
     DenseMap<StringRef, FunctionImporter::ExportSetTy> *ExportLists) {
-  if (WorkloadDefinitions.empty() && ContextualProfile.empty()) {
+  if (WorkloadDefinitions.empty() && UseCtxProfile.empty()) {
     LLVM_DEBUG(dbgs() << "[Workload] Using the regular imports manager.\n");
     return std::unique_ptr<ModuleImportsManager>(
         new ModuleImportsManager(IsPrevailing, Index, ExportLists));
diff --git a/llvm/test/ThinLTO/X86/ctxprof.ll b/llvm/test/ThinLTO/X86/ctxprof.ll
index 4c86ec9f4c4790..5c1f5f99daa161 100644
--- a/llvm/test/ThinLTO/X86/ctxprof.ll
+++ b/llvm/test/ThinLTO/X86/ctxprof.ll
@@ -46,7 +46,7 @@
 ; RUN: llvm-ctxprof-util fromJSON --input %t_exp/ctxprof.json --output %t_exp/ctxprof.bitstream
 ; RUN: llvm-lto2 run %t/m1.bc %t/m2.bc \
 ; RUN:  -o %t_exp/result.o -save-temps \
-; RUN:  -thinlto-pgo-ctx-prof=%t_exp/ctxprof.bitstream \
+; RUN:  -use-ctx-profile=%t_exp/ctxprof.bitstream \
 ; RUN:  -r %t/m1.bc,m1_f1,plx \
 ; RUN:  -r %t/m2.bc,m2_f1,plx
 ; RUN: llvm-dis %t_exp/result.o.1.3.import.bc -o - | FileCheck %s --check-prefix=FIRST

@mtrofin mtrofin merged commit 6807ca8 into llvm:main Aug 13, 2024
8 of 10 checks passed
@mtrofin mtrofin deleted the one_flag branch August 14, 2024 14:57
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Aug 22, 2024
reverts: [nfc][ctx_prof] Use one flag for the "use" scenario (llvm#103377)
Change-Id: Ia1cd78370ab5e496695d5a56738b9c4927f15e0f
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Sep 7, 2024
Change-Id: I413faf1de66b794b329fbab5170deb319f1f5173
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.

2 participants