[clang] 11a6799 - [clang][CodeGen] Omit pre-opt link when post-opt is link requested (#85672)
via cfe-commits
cfe-commits at lists.llvm.org
Wed May 8 08:11:19 PDT 2024
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
More information about the cfe-commits
mailing list