[clang] [clang][CodeGen] Omit pre-opt link when post-opt is link requested (PR #85672)
Jacob Lambert via cfe-commits
cfe-commits at lists.llvm.org
Thu May 2 14:56:41 PDT 2024
https://github.com/lamb-j updated https://github.com/llvm/llvm-project/pull/85672
>From aff1a762a73ce30cde38a6fcbbed8a3e4f0b5366 Mon Sep 17 00:00:00 2001
From: Jacob Lambert <jacob.lambert at amd.com>
Date: Mon, 18 Mar 2024 10:19:38 -0700
Subject: [PATCH 1/6] [clang][CodeGen] Omit pre-opt link when post-opt link
requested
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:
-link-builtin-bitcodes-postopt
---
clang/lib/CodeGen/BackendUtil.cpp | 13 +++++--------
clang/lib/CodeGen/CodeGenAction.cpp | 6 +++---
clang/lib/CodeGen/LinkInModulesPass.cpp | 4 ----
3 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 82b30b8d815629..c5571eb15ce3a9 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -112,9 +112,9 @@ static cl::opt<bool> ClSanitizeOnOptimizerEarlyEP(
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."), cl::init(false));
+cl::opt<bool> ClLinkBuiltinBitcodePostopt(
+ "link-builtin-bitcode-postopt", cl::Optional,
+ cl::desc("Link builtin bitcodes after optimization."), cl::init(false));
} // namespace llvm
namespace {
@@ -1051,11 +1051,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 (ClLinkBuiltinBitcodePostopt)
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 bb9aaba025fa59..51f54d178672d1 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -58,7 +58,7 @@ using namespace llvm;
#define DEBUG_TYPE "codegenaction"
namespace llvm {
-extern cl::opt<bool> ClRelinkBuiltinBitcodePostop;
+extern cl::opt<bool> ClLinkBuiltinBitcodePostopt;
}
namespace clang {
@@ -359,7 +359,7 @@ void BackendConsumer::HandleTranslationUnit(ASTContext &C) {
}
// Link each LinkModule into our module.
- if (LinkInModules(getModule()))
+ if (!LinkBuiltinBitcodesPostopt && LinkInModules(getModule()))
return;
for (auto &F : getModule()->functions()) {
@@ -1213,7 +1213,7 @@ void CodeGenAction::ExecuteAction() {
std::move(LinkModules), *VMContext, nullptr);
// Link in each pending link module.
- if (Result.LinkInModules(&*TheModule))
+ if (!LinkBuiltinBitcodesPostopt && 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 929539cc8f3346..ce1c0ebe046a61 100644
--- a/clang/lib/CodeGen/LinkInModulesPass.cpp
+++ b/clang/lib/CodeGen/LinkInModulesPass.cpp
@@ -28,10 +28,6 @@ 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!");
>From 9a6757c9497019dbc2c9c0d449c0d1cbc70c98fd Mon Sep 17 00:00:00 2001
From: Jacob Lambert <jacob.lambert at amd.com>
Date: Mon, 18 Mar 2024 10:47:25 -0700
Subject: [PATCH 2/6] fix option name
---
clang/lib/CodeGen/CodeGenAction.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 51f54d178672d1..a3ff5dc7d29e99 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -359,7 +359,7 @@ void BackendConsumer::HandleTranslationUnit(ASTContext &C) {
}
// Link each LinkModule into our module.
- if (!LinkBuiltinBitcodesPostopt && LinkInModules(getModule()))
+ if (!ClLinkBuiltinBitcodePostopt && LinkInModules(getModule()))
return;
for (auto &F : getModule()->functions()) {
@@ -1213,7 +1213,7 @@ void CodeGenAction::ExecuteAction() {
std::move(LinkModules), *VMContext, nullptr);
// Link in each pending link module.
- if (!LinkBuiltinBitcodesPostopt && Result.LinkInModules(&*TheModule))
+ if (!ClLinkBuiltinBitcodePostopt && Result.LinkInModules(&*TheModule))
return;
// PR44896: Force DiscardValueNames as false. DiscardValueNames cannot be
>From e2b6956227c40438b39d431333bcae9d27bdfd64 Mon Sep 17 00:00:00 2001
From: Jacob Lambert <jacob.lambert at amd.com>
Date: Mon, 18 Mar 2024 11:37:00 -0700
Subject: [PATCH 3/6] Fix analyses return and typo
---
clang/lib/CodeGen/LinkInModulesPass.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/LinkInModulesPass.cpp b/clang/lib/CodeGen/LinkInModulesPass.cpp
index ce1c0ebe046a61..c3831aae13b647 100644
--- a/clang/lib/CodeGen/LinkInModulesPass.cpp
+++ b/clang/lib/CodeGen/LinkInModulesPass.cpp
@@ -29,7 +29,7 @@ PreservedAnalyses LinkInModulesPass::run(Module &M, ModuleAnalysisManager &AM) {
return PreservedAnalyses::all();
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();
}
>From 2238bf93af3ec7b3f0cbfd384148d96c9ee3039f Mon Sep 17 00:00:00 2001
From: Jacob Lambert <jacob.lambert at amd.com>
Date: Mon, 18 Mar 2024 11:38:31 -0700
Subject: [PATCH 4/6] Update comment
---
clang/lib/CodeGen/BackendUtil.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index c5571eb15ce3a9..4031dc6b4db6cd 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -111,7 +111,7 @@ static cl::opt<bool> ClSanitizeOnOptimizerEarlyEP(
extern cl::opt<InstrProfCorrelator::ProfCorrelatorKind> ProfileCorrelate;
-// Re-link builtin bitcodes after optimization
+// Link builtin bitcodes after optimization
cl::opt<bool> ClLinkBuiltinBitcodePostopt(
"link-builtin-bitcode-postopt", cl::Optional,
cl::desc("Link builtin bitcodes after optimization."), cl::init(false));
>From 064744af2414c14f3fd71128c4d511c2dfe0c04c Mon Sep 17 00:00:00 2001
From: Jacob Lambert <jacob.lambert at amd.com>
Date: Tue, 2 Apr 2024 11:16:41 -0700
Subject: [PATCH 5/6] Remove ReloadModules() declaration/definition
---
clang/lib/CodeGen/BackendConsumer.h | 4 ----
clang/lib/CodeGen/CodeGenAction.cpp | 29 -----------------------------
2 files changed, 33 deletions(-)
diff --git a/clang/lib/CodeGen/BackendConsumer.h b/clang/lib/CodeGen/BackendConsumer.h
index fd0f1984d6c0f7..f9edbe901bb850 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/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index a3ff5dc7d29e99..ae46fc45582f88 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -229,35 +229,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) {
>From 2dc99c25bf20edef31e7057f89c8603d7134e9ae Mon Sep 17 00:00:00 2001
From: Jacob Lambert <jacob.lambert at amd.com>
Date: Thu, 2 May 2024 14:36:10 -0700
Subject: [PATCH 6/6] Switch to a regular Clang option, and add a LIT test
---
clang/include/clang/Basic/CodeGenOptions.def | 1 +
clang/include/clang/Driver/Options.td | 5 +++
clang/lib/CodeGen/BackendUtil.cpp | 7 +---
clang/lib/CodeGen/CodeGenAction.cpp | 8 ++---
clang/test/Driver/linking-bitcode-postopt.cl | 37 ++++++++++++++++++++
5 files changed, 46 insertions(+), 12 deletions(-)
create mode 100644 clang/test/Driver/linking-bitcode-postopt.cl
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 340b08dd7e2a33..3f64c82c88e691 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -308,6 +308,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 2801560ddaf648..5f62a8724397ae 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -7087,6 +7087,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/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index a348b21c4905f6..a0f1aa2390b22a 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;
-
-// Link builtin bitcodes after optimization
-cl::opt<bool> ClLinkBuiltinBitcodePostopt(
- "link-builtin-bitcode-postopt", cl::Optional,
- cl::desc("Link builtin bitcodes after optimization."));
} // namespace llvm
namespace {
@@ -1055,7 +1050,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
}
// Link against bitcodes supplied via the -mlink-builtin-bitcode option
- if (ClLinkBuiltinBitcodePostopt)
+ 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 7f266bcda4ddec..0255f05b1f90e5 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> ClLinkBuiltinBitcodePostopt;
-}
-
namespace clang {
class BackendConsumer;
class ClangDiagnosticHandler final : public DiagnosticHandler {
@@ -333,7 +329,7 @@ void BackendConsumer::HandleTranslationUnit(ASTContext &C) {
}
// Link each LinkModule into our module.
- if (!ClLinkBuiltinBitcodePostopt && LinkInModules(getModule()))
+ if (!CodeGenOpts.LinkBitcodePostopt && LinkInModules(getModule()))
return;
for (auto &F : getModule()->functions()) {
@@ -1203,7 +1199,7 @@ void CodeGenAction::ExecuteAction() {
std::move(LinkModules), *VMContext, nullptr);
// Link in each pending link module.
- if (!ClLinkBuiltinBitcodePostopt && Result.LinkInModules(&*TheModule))
+ if (!CodeGenOpts.LinkBitcodePostopt && Result.LinkInModules(&*TheModule))
return;
// PR44896: Force DiscardValueNames as false. DiscardValueNames cannot be
diff --git a/clang/test/Driver/linking-bitcode-postopt.cl b/clang/test/Driver/linking-bitcode-postopt.cl
new file mode 100644
index 00000000000000..b70a3c538fc15a
--- /dev/null
+++ b/clang/test/Driver/linking-bitcode-postopt.cl
@@ -0,0 +1,37 @@
+// REQUIRES: amdgpu-registered-target
+
+// Test that -mlink-bitcode-postopt correctly enables LinkInModulesPass
+
+// RUN: %clang -c -mllvm -print-pipeline-passes -target amdgcn-amd-amdhsa \
+// RUN: -x cl -mcpu=gfx900 \
+// RUN: %s \
+// RUN: 2>&1 | FileCheck --check-prefixes=DEFAULT %s
+
+// DEFAULT-NOT: LinkInModulesPass
+
+// RUN: %clang -c -mllvm -print-pipeline-passes -target amdgcn-amd-amdhsa \
+// RUN: -x cl -mcpu=gfx900 \
+// RUN: -Xclang -mlink-builtin-bitcode-postopt \
+// RUN: %s \
+// RUN: 2>&1 | FileCheck --check-prefixes=OPTION-POSITIVE %s
+
+// OPTION-POSITIVE: LinkInModulesPass
+
+// RUN: %clang -c -mllvm -print-pipeline-passes -target amdgcn-amd-amdhsa \
+// RUN: -x cl -mcpu=gfx900 \
+// RUN: -Xclang -mno-link-builtin-bitcode-postopt \
+// RUN: %s \
+// RUN: 2>&1 | FileCheck --check-prefixes=OPTION-NEGATIVE %s
+
+// OPTION-NEGATIVE-NOT: LinkInModulesPass
+
+// RUN: %clang -c -mllvm -print-pipeline-passes -target amdgcn-amd-amdhsa \
+// RUN: -x cl -mcpu=gfx900 \
+// RUN: -Xclang -mlink-builtin-bitcode-postopt \
+// RUN: -Xclang -mno-link-builtin-bitcode-postopt \
+// RUN: %s \
+// RUN: 2>&1 | FileCheck --check-prefixes=OPTION-POSITIVE-NEGATIVE %s
+
+// OPTION-POSITIVE-NEGATIVE-NOT: LinkInModulesPass
+
+kernel void func(void);
More information about the cfe-commits
mailing list