Skip to content
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

[LTO] Remove Config.UseDefaultPipeline #82587

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

igorkudrin
Copy link
Collaborator

This option is not used. It was added in D122133, 5856f30, with the only usage in ClangLinkerWrapper.cpp, which was later updated in a1d57fc, and then finally removed in D142650, 6185246.

This option is not used. It was added in https://reviews.llvm.org/D122133,
with the only usage in `ClangLinkerWrapper.cpp`, which was later removed
in https://reviews.llvm.org/D142650.
@igorkudrin igorkudrin added the LTO Link time optimization (regular/full LTO or ThinLTO) label Feb 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 22, 2024

@llvm/pr-subscribers-lto

Author: Igor Kudrin (igorkudrin)

Changes

This option is not used. It was added in D122133, 5856f30, with the only usage in ClangLinkerWrapper.cpp, which was later updated in a1d57fc, and then finally removed in D142650, 6185246.


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

2 Files Affected:

  • (modified) llvm/include/llvm/LTO/Config.h (-3)
  • (modified) llvm/lib/LTO/LTOBackend.cpp (-2)
diff --git a/llvm/include/llvm/LTO/Config.h b/llvm/include/llvm/LTO/Config.h
index 6fb55f1cf1686a..482b6e55a19d35 100644
--- a/llvm/include/llvm/LTO/Config.h
+++ b/llvm/include/llvm/LTO/Config.h
@@ -60,9 +60,6 @@ struct Config {
   bool VerifyEach = false;
   bool DisableVerify = false;
 
-  /// Use the standard optimization pipeline.
-  bool UseDefaultPipeline = false;
-
   /// Flag to indicate that the optimizer should not assume builtins are present
   /// on the target.
   bool Freestanding = false;
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 7b3a7590dfa743..6cfe67779b1a7d 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -330,8 +330,6 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
       report_fatal_error(Twine("unable to parse pass pipeline description '") +
                          Conf.OptPipeline + "': " + toString(std::move(Err)));
     }
-  } else if (Conf.UseDefaultPipeline) {
-    MPM.addPass(PB.buildPerModuleDefaultPipeline(OL));
   } else if (IsThinLTO) {
     MPM.addPass(PB.buildThinLTODefaultPipeline(OL, ImportSummary));
   } else {

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

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

Yeah this was to fix a large performance regression in some application on the short term but has since been fixed correctly. Thanks for catching this.

@igorkudrin igorkudrin merged commit ec24094 into llvm:main Feb 22, 2024
6 checks passed
@igorkudrin igorkudrin deleted the lto-remove-Conf-UseDefaultPipeline branch February 22, 2024 18:05
nicolasmarie2 pushed a commit to nicolasmarie2/llvm-project that referenced this pull request Apr 2, 2024
This reverts commit ec24094.
We do need Config.UseDefaultPipeline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTO Link time optimization (regular/full LTO or ThinLTO)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants