Author: Jacob Lambert Date: 2024-05-08T08:11:15-07:00 New Revision: 11a6799740f824282650aa9ec249b55dcf1a8aae
URL: https://github.com/llvm/llvm-project/commit/11a6799740f824282650aa9ec249b55dcf1a8aae DIFF: https://github.com/llvm/llvm-project/commit/11a6799740f824282650aa9ec249b55dcf1a8aae.diff LOG: [clang][CodeGen] Omit pre-opt link when post-opt is link requested (#85672) Currently, when the -relink-builtin-bitcodes-postop option is used we link builtin bitcodes twice: once before optimization, and again after optimization. With this change, we omit the pre-opt linking when the option is set, and we rename the option to the following: -Xclang -mlink-builtin-bitcodes-postopt (-Xclang -mno-link-builtin-bitcodes-postopt) The goal of this change is to reduce compile time. We do lose the theoretical benefits of pre-opt linking, but in practice these are small than the overhead of linking twice. However we may be able to address this in a future patch by adjusting the position of the builtin-bitcode linking pass. Compilations not setting the option are unaffected Added: clang/test/CodeGen/linking-bitcode-postopt.cpp Modified: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Driver/Options.td clang/lib/CodeGen/BackendConsumer.h clang/lib/CodeGen/BackendUtil.cpp clang/lib/CodeGen/CodeGenAction.cpp clang/lib/CodeGen/LinkInModulesPass.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def index b964e45574782..07b0ca1691a67 100644 --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -309,6 +309,7 @@ CODEGENOPT(UnrollLoops , 1, 0) ///< Control whether loops are unrolled. CODEGENOPT(RerollLoops , 1, 0) ///< Control whether loops are rerolled. CODEGENOPT(NoUseJumpTables , 1, 0) ///< Set when -fno-jump-tables is enabled. VALUE_CODEGENOPT(UnwindTables, 2, 0) ///< Unwind tables (1) or asynchronous unwind tables (2) +CODEGENOPT(LinkBitcodePostopt, 1, 0) ///< Link builtin bitcodes after optimization pipeline. CODEGENOPT(VectorizeLoop , 1, 0) ///< Run loop vectorizer. CODEGENOPT(VectorizeSLP , 1, 0) ///< Run SLP vectorizer. CODEGENOPT(ProfileSampleAccurate, 1, 0) ///< Sample profile is accurate. diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 734ae7833f5c3..322cc12af34ac 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -7092,6 +7092,11 @@ def mlink_bitcode_file : Separate<["-"], "mlink-bitcode-file">, def mlink_builtin_bitcode : Separate<["-"], "mlink-builtin-bitcode">, HelpText<"Link and internalize needed symbols from the given bitcode file " "before performing optimizations.">; +defm link_builtin_bitcode_postopt: BoolMOption<"link-builtin-bitcode-postopt", + CodeGenOpts<"LinkBitcodePostopt">, DefaultFalse, + PosFlag<SetTrue, [], [ClangOption], "Link builtin bitcodes after the " + "optimization pipeline">, + NegFlag<SetFalse, [], [ClangOption]>>; def vectorize_loops : Flag<["-"], "vectorize-loops">, HelpText<"Run the Loop vectorization passes">, MarshallingInfoFlag<CodeGenOpts<"VectorizeLoop">>; diff --git a/clang/lib/CodeGen/BackendConsumer.h b/clang/lib/CodeGen/BackendConsumer.h index fd0f1984d6c0f..f9edbe901bb85 100644 --- a/clang/lib/CodeGen/BackendConsumer.h +++ b/clang/lib/CodeGen/BackendConsumer.h @@ -115,10 +115,6 @@ class BackendConsumer : public ASTConsumer { // Links each entry in LinkModules into our module. Returns true on error. bool LinkInModules(llvm::Module *M, bool ShouldLinkFiles = true); - // Load a bitcode module from -mlink-builtin-bitcode option using - // methods from a BackendConsumer instead of CompilerInstance - bool ReloadModules(llvm::Module *M); - /// Get the best possible source location to represent a diagnostic that /// may have associated debug info. const FullSourceLoc getBestLocationFromDebugLoc( diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 119ec4704002c..90985c08fe7f8 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -120,11 +120,6 @@ static cl::opt<PGOOptions::ColdFuncOpt> ClPGOColdFuncAttr( "Mark cold functions with optnone."))); extern cl::opt<InstrProfCorrelator::ProfCorrelatorKind> ProfileCorrelate; - -// Re-link builtin bitcodes after optimization -cl::opt<bool> ClRelinkBuiltinBitcodePostop( - "relink-builtin-bitcode-postop", cl::Optional, - cl::desc("Re-link builtin bitcodes after optimization.")); } // namespace llvm namespace { @@ -1055,11 +1050,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( } } - // Re-link against any bitcodes supplied via the -mlink-builtin-bitcode option - // Some optimizations may generate new function calls that would not have - // been linked pre-optimization (i.e. fused sincos calls generated by - // AMDGPULibCalls::fold_sincos.) - if (ClRelinkBuiltinBitcodePostop) + // Link against bitcodes supplied via the -mlink-builtin-bitcode option + if (CodeGenOpts.LinkBitcodePostopt) MPM.addPass(LinkInModulesPass(BC, false)); // Add a verifier pass if requested. We don't have to do this if the action diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index 1a6b628016f74..0255f05b1f90e 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -60,10 +60,6 @@ using namespace llvm; #define DEBUG_TYPE "codegenaction" -namespace llvm { -extern cl::opt<bool> ClRelinkBuiltinBitcodePostop; -} - namespace clang { class BackendConsumer; class ClangDiagnosticHandler final : public DiagnosticHandler { @@ -232,35 +228,6 @@ void BackendConsumer::HandleInterestingDecl(DeclGroupRef D) { HandleTopLevelDecl(D); } -bool BackendConsumer::ReloadModules(llvm::Module *M) { - for (const CodeGenOptions::BitcodeFileToLink &F : - CodeGenOpts.LinkBitcodeFiles) { - auto BCBuf = FileMgr.getBufferForFile(F.Filename); - if (!BCBuf) { - Diags.Report(diag::err_cannot_open_file) - << F.Filename << BCBuf.getError().message(); - LinkModules.clear(); - return true; - } - - LLVMContext &Ctx = getModule()->getContext(); - Expected<std::unique_ptr<llvm::Module>> ModuleOrErr = - getOwningLazyBitcodeModule(std::move(*BCBuf), Ctx); - - if (!ModuleOrErr) { - handleAllErrors(ModuleOrErr.takeError(), [&](ErrorInfoBase &EIB) { - Diags.Report(diag::err_cannot_open_file) << F.Filename << EIB.message(); - }); - LinkModules.clear(); - return true; - } - LinkModules.push_back({std::move(ModuleOrErr.get()), F.PropagateAttrs, - F.Internalize, F.LinkFlags}); - } - - return false; // success -} - // Links each entry in LinkModules into our module. Returns true on error. bool BackendConsumer::LinkInModules(llvm::Module *M, bool ShouldLinkFiles) { for (auto &LM : LinkModules) { @@ -362,7 +329,7 @@ void BackendConsumer::HandleTranslationUnit(ASTContext &C) { } // Link each LinkModule into our module. - if (LinkInModules(getModule())) + if (!CodeGenOpts.LinkBitcodePostopt && LinkInModules(getModule())) return; for (auto &F : getModule()->functions()) { @@ -1232,7 +1199,7 @@ void CodeGenAction::ExecuteAction() { std::move(LinkModules), *VMContext, nullptr); // Link in each pending link module. - if (Result.LinkInModules(&*TheModule)) + if (!CodeGenOpts.LinkBitcodePostopt && Result.LinkInModules(&*TheModule)) return; // PR44896: Force DiscardValueNames as false. DiscardValueNames cannot be diff --git a/clang/lib/CodeGen/LinkInModulesPass.cpp b/clang/lib/CodeGen/LinkInModulesPass.cpp index 929539cc8f334..c3831aae13b64 100644 --- a/clang/lib/CodeGen/LinkInModulesPass.cpp +++ b/clang/lib/CodeGen/LinkInModulesPass.cpp @@ -28,12 +28,8 @@ PreservedAnalyses LinkInModulesPass::run(Module &M, ModuleAnalysisManager &AM) { if (!BC) return PreservedAnalyses::all(); - // Re-load bitcode modules from files - if (BC->ReloadModules(&M)) - report_fatal_error("Bitcode module re-loading failed, aborted!"); - if (BC->LinkInModules(&M, ShouldLinkFiles)) - report_fatal_error("Bitcode module re-linking failed, aborted!"); + report_fatal_error("Bitcode module postopt linking failed, aborted!"); - return PreservedAnalyses::all(); + return PreservedAnalyses::none(); } diff --git a/clang/test/CodeGen/linking-bitcode-postopt.cpp b/clang/test/CodeGen/linking-bitcode-postopt.cpp new file mode 100644 index 0000000000000..a0486ed0c9a83 --- /dev/null +++ b/clang/test/CodeGen/linking-bitcode-postopt.cpp @@ -0,0 +1,31 @@ +// REQUIRES: amdgpu-registered-target + +// Test that -mlink-bitcode-postopt correctly enables LinkInModulesPass + +// RUN: %clang_cc1 -triple amdgcn-- -emit-llvm-bc -o /dev/null \ +// RUN: -mllvm -print-pipeline-passes \ +// RUN: %s 2>&1 | FileCheck --check-prefixes=DEFAULT %s + +// DEFAULT-NOT: LinkInModulesPass + +// RUN: %clang_cc1 -triple amdgcn-- -emit-llvm-bc -o /dev/null \ +// RUN: -mllvm -print-pipeline-passes \ +// RUN: -mlink-builtin-bitcode-postopt \ +// RUN: %s 2>&1 | FileCheck --check-prefixes=OPTION-POSITIVE %s + +// OPTION-POSITIVE: LinkInModulesPass + +// RUN: %clang_cc1 -triple amdgcn-- -emit-llvm-bc -o /dev/null \ +// RUN: -mllvm -print-pipeline-passes \ +// RUN: -mno-link-builtin-bitcode-postopt \ +// RUN: %s 2>&1 | FileCheck --check-prefixes=OPTION-NEGATIVE %s + +// OPTION-NEGATIVE-NOT: LinkInModulesPass + +// RUN: %clang_cc1 -triple amdgcn-- -emit-llvm-bc -o /dev/null \ +// RUN: -mllvm -print-pipeline-passes \ +// RUN: -mlink-builtin-bitcode-postopt \ +// RUN: -mno-link-builtin-bitcode-postopt \ +// RUN: %s 2>&1 | FileCheck --check-prefixes=OPTION-POSITIVE-NEGATIVE %s + +// OPTION-POSITIVE-NEGATIVE-NOT: LinkInModulesPass _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
