[clang][NFC] Make OffloadLTOMode getter a separate method #101200
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-amdgpu Author: None (macurtis-amd) ChangesIMO, improves readability and makes it easier to find the places where we are getting the offload lto mode. Full diff: https://github.com/llvm/llvm-project/pull/101200.diff 7 Files Affected:
diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index 04b46782467d6..84eadd42880a5 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -715,14 +715,16 @@ class Driver {
ModuleHeaderMode getModuleHeaderMode() const { return CXX20HeaderType; }
/// Returns true if we are performing any kind of LTO.
- bool isUsingLTO(bool IsOffload = false) const {
- return getLTOMode(IsOffload) != LTOK_None;
- }
+ bool isUsingLTO() const { return getLTOMode() != LTOK_None; }
/// Get the specific kind of LTO being performed.
- LTOKind getLTOMode(bool IsOffload = false) const {
- return IsOffload ? OffloadLTOMode : LTOMode;
- }
+ LTOKind getLTOMode() const { return LTOMode; }
+
+ /// Returns true if we are performing any kind of offload LTO.
+ bool isUsingOffloadLTO() const { return getOffloadLTOMode() != LTOK_None; }
+
+ /// Get the specific kind of offload LTO being performed.
+ LTOKind getOffloadLTOMode() const { return OffloadLTOMode; }
private:
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 8e44d5afa40e0..8a5f83edfe081 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -3492,7 +3492,7 @@ class OffloadingActionBuilder final {
// a fat binary containing all the code objects for different GPU's.
// The fat binary is then an input to the host action.
for (unsigned I = 0, E = GpuArchList.size(); I != E; ++I) {
- if (C.getDriver().isUsingLTO(/*IsOffload=*/true)) {
+ if (C.getDriver().isUsingOffloadLTO()) {
// When LTO is enabled, skip the backend and assemble phases and
// use lld to link the bitcode.
ActionList AL;
@@ -4856,8 +4856,7 @@ Action *Driver::ConstructPhaseAction(
Output = types::TY_LTO_BC;
return C.MakeAction<BackendJobAction>(Input, Output);
}
- if (isUsingLTO(/* IsOffload */ true) &&
- TargetDeviceOffloadKind != Action::OFK_None) {
+ if (isUsingOffloadLTO() && TargetDeviceOffloadKind != Action::OFK_None) {
types::ID Output =
Args.hasArg(options::OPT_S) ? types::TY_LTO_IR : types::TY_LTO_BC;
return C.MakeAction<BackendJobAction>(Input, Output);
diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index b75d400e6ce91..d43e683e46852 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -57,7 +57,7 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
}
// Link the bitcode library late if we're using device LTO.
- if (getDriver().isUsingLTO(/* IsOffload */ true))
+ if (getDriver().isUsingOffloadLTO())
return;
}
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 843d68c85bc59..6d2f3982d5faa 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4959,8 +4959,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
bool IsRDCMode =
Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false);
- bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
- auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
+
+ auto LTOMode = IsDeviceOffloadAction ? D.getOffloadLTOMode() : D.getLTOMode();
+ bool IsUsingLTO = LTOMode != LTOK_None;
// Extract API doesn't have a main input file, so invent a fake one as a
// placeholder.
@@ -7835,7 +7836,12 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
// were suppressed because this is not the device offload action.
// Check if we are using PS4 in regular LTO mode.
// Otherwise, issue an error.
- if ((!IsUsingLTO && !D.isUsingLTO(!IsDeviceOffloadAction)) ||
+
+ auto OtherLTOMode =
+ IsDeviceOffloadAction ? D.getLTOMode() : D.getOffloadLTOMode();
+ auto OtherIsUsingLTO = OtherLTOMode != LTOK_None;
+
+ if ((!IsUsingLTO && !OtherIsUsingLTO) ||
(IsPS4 && !UnifiedLTO && (D.getLTOMode() != LTOK_Full)))
D.Diag(diag::err_drv_argument_only_allowed_with)
<< "-fwhole-program-vtables"
@@ -9032,7 +9038,7 @@ void OffloadPackager::ConstructJob(Compilation &C, const JobAction &JA,
"kind=" + Kind.str(),
};
- if (TC->getDriver().isUsingLTO(/* IsOffload */ true) ||
+ if (TC->getDriver().isUsingOffloadLTO() ||
TC->getTriple().isAMDGPU())
for (StringRef Feature : FeatureArgs)
Parts.emplace_back("feature=" + Feature.str());
diff --git a/clang/lib/Driver/ToolChains/Cuda.cpp b/clang/lib/Driver/ToolChains/Cuda.cpp
index e98e574d6cc2b..c01269a65d969 100644
--- a/clang/lib/Driver/ToolChains/Cuda.cpp
+++ b/clang/lib/Driver/ToolChains/Cuda.cpp
@@ -885,7 +885,7 @@ void CudaToolChain::addClangTargetOptions(
}
// Link the bitcode library late if we're using device LTO.
- if (getDriver().isUsingLTO(/* IsOffload */ true))
+ if (getDriver().isUsingOffloadLTO())
return;
addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GpuArch.str(),
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index f5de5eb23e4be..17d9aa932f310 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -734,7 +734,7 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("-fcolor-diagnostics");
// LTO mode is parsed by the Clang driver library.
- LTOKind LTOMode = D.getLTOMode(/* IsOffload */ false);
+ LTOKind LTOMode = D.getLTOMode();
assert(LTOMode != LTOK_Unknown && "Unknown LTO mode.");
if (LTOMode == LTOK_Full)
CmdArgs.push_back("-flto=full");
diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp
index c35b0febb262d..cbb8fab69a316 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -120,7 +120,7 @@ void AMDGCN::Linker::constructLldCommand(Compilation &C, const JobAction &JA,
auto &TC = getToolChain();
auto &D = TC.getDriver();
assert(!Inputs.empty() && "Must have at least one input.");
- bool IsThinLTO = D.getLTOMode(/*IsOffload=*/true) == LTOK_Thin;
+ bool IsThinLTO = D.getOffloadLTOMode() == LTOK_Thin;
addLTOOptions(TC, Args, LldArgs, Output, Inputs[0], IsThinLTO);
// Extract all the -m options
|
|
@llvm/pr-subscribers-clang-driver Author: None (macurtis-amd) ChangesIMO, improves readability and makes it easier to find the places where we are getting the offload lto mode. Full diff: https://github.com/llvm/llvm-project/pull/101200.diff 7 Files Affected:
diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index 04b46782467d6..84eadd42880a5 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -715,14 +715,16 @@ class Driver {
ModuleHeaderMode getModuleHeaderMode() const { return CXX20HeaderType; }
/// Returns true if we are performing any kind of LTO.
- bool isUsingLTO(bool IsOffload = false) const {
- return getLTOMode(IsOffload) != LTOK_None;
- }
+ bool isUsingLTO() const { return getLTOMode() != LTOK_None; }
/// Get the specific kind of LTO being performed.
- LTOKind getLTOMode(bool IsOffload = false) const {
- return IsOffload ? OffloadLTOMode : LTOMode;
- }
+ LTOKind getLTOMode() const { return LTOMode; }
+
+ /// Returns true if we are performing any kind of offload LTO.
+ bool isUsingOffloadLTO() const { return getOffloadLTOMode() != LTOK_None; }
+
+ /// Get the specific kind of offload LTO being performed.
+ LTOKind getOffloadLTOMode() const { return OffloadLTOMode; }
private:
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 8e44d5afa40e0..8a5f83edfe081 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -3492,7 +3492,7 @@ class OffloadingActionBuilder final {
// a fat binary containing all the code objects for different GPU's.
// The fat binary is then an input to the host action.
for (unsigned I = 0, E = GpuArchList.size(); I != E; ++I) {
- if (C.getDriver().isUsingLTO(/*IsOffload=*/true)) {
+ if (C.getDriver().isUsingOffloadLTO()) {
// When LTO is enabled, skip the backend and assemble phases and
// use lld to link the bitcode.
ActionList AL;
@@ -4856,8 +4856,7 @@ Action *Driver::ConstructPhaseAction(
Output = types::TY_LTO_BC;
return C.MakeAction<BackendJobAction>(Input, Output);
}
- if (isUsingLTO(/* IsOffload */ true) &&
- TargetDeviceOffloadKind != Action::OFK_None) {
+ if (isUsingOffloadLTO() && TargetDeviceOffloadKind != Action::OFK_None) {
types::ID Output =
Args.hasArg(options::OPT_S) ? types::TY_LTO_IR : types::TY_LTO_BC;
return C.MakeAction<BackendJobAction>(Input, Output);
diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index b75d400e6ce91..d43e683e46852 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -57,7 +57,7 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
}
// Link the bitcode library late if we're using device LTO.
- if (getDriver().isUsingLTO(/* IsOffload */ true))
+ if (getDriver().isUsingOffloadLTO())
return;
}
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 843d68c85bc59..6d2f3982d5faa 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4959,8 +4959,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
bool IsRDCMode =
Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false);
- bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
- auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
+
+ auto LTOMode = IsDeviceOffloadAction ? D.getOffloadLTOMode() : D.getLTOMode();
+ bool IsUsingLTO = LTOMode != LTOK_None;
// Extract API doesn't have a main input file, so invent a fake one as a
// placeholder.
@@ -7835,7 +7836,12 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
// were suppressed because this is not the device offload action.
// Check if we are using PS4 in regular LTO mode.
// Otherwise, issue an error.
- if ((!IsUsingLTO && !D.isUsingLTO(!IsDeviceOffloadAction)) ||
+
+ auto OtherLTOMode =
+ IsDeviceOffloadAction ? D.getLTOMode() : D.getOffloadLTOMode();
+ auto OtherIsUsingLTO = OtherLTOMode != LTOK_None;
+
+ if ((!IsUsingLTO && !OtherIsUsingLTO) ||
(IsPS4 && !UnifiedLTO && (D.getLTOMode() != LTOK_Full)))
D.Diag(diag::err_drv_argument_only_allowed_with)
<< "-fwhole-program-vtables"
@@ -9032,7 +9038,7 @@ void OffloadPackager::ConstructJob(Compilation &C, const JobAction &JA,
"kind=" + Kind.str(),
};
- if (TC->getDriver().isUsingLTO(/* IsOffload */ true) ||
+ if (TC->getDriver().isUsingOffloadLTO() ||
TC->getTriple().isAMDGPU())
for (StringRef Feature : FeatureArgs)
Parts.emplace_back("feature=" + Feature.str());
diff --git a/clang/lib/Driver/ToolChains/Cuda.cpp b/clang/lib/Driver/ToolChains/Cuda.cpp
index e98e574d6cc2b..c01269a65d969 100644
--- a/clang/lib/Driver/ToolChains/Cuda.cpp
+++ b/clang/lib/Driver/ToolChains/Cuda.cpp
@@ -885,7 +885,7 @@ void CudaToolChain::addClangTargetOptions(
}
// Link the bitcode library late if we're using device LTO.
- if (getDriver().isUsingLTO(/* IsOffload */ true))
+ if (getDriver().isUsingOffloadLTO())
return;
addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GpuArch.str(),
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index f5de5eb23e4be..17d9aa932f310 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -734,7 +734,7 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("-fcolor-diagnostics");
// LTO mode is parsed by the Clang driver library.
- LTOKind LTOMode = D.getLTOMode(/* IsOffload */ false);
+ LTOKind LTOMode = D.getLTOMode();
assert(LTOMode != LTOK_Unknown && "Unknown LTO mode.");
if (LTOMode == LTOK_Full)
CmdArgs.push_back("-flto=full");
diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp
index c35b0febb262d..cbb8fab69a316 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -120,7 +120,7 @@ void AMDGCN::Linker::constructLldCommand(Compilation &C, const JobAction &JA,
auto &TC = getToolChain();
auto &D = TC.getDriver();
assert(!Inputs.empty() && "Must have at least one input.");
- bool IsThinLTO = D.getLTOMode(/*IsOffload=*/true) == LTOK_Thin;
+ bool IsThinLTO = D.getOffloadLTOMode() == LTOK_Thin;
addLTOOptions(TC, Args, LldArgs, Output, Inputs[0], IsThinLTO);
// Extract all the -m options
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
IMO, improves readability and makes it easier to find the places where we are getting the offload lto mode.
macurtis-amd
force-pushed
the
offload-lto-mode-sep
branch
from
72a4391 to
8af6a8f
Compare
|
@yxsamliu Would you mind reviewing this change? |
jhuber6
approved these changes
Aug 5, 2024
Sorry for the delay. LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Minor readability improvement (IMHO). Also makes it easier to find the places where we are getting the offload lto mode.