Skip to content

Commit 073460a

Browse files
jchlandajchlanda
authored
[HIP][Clang][Driver] Move BC preference logic into ROCm detection (#149294)
This patch provides a single point for handling the logic behind choosing common bitcode libraries. The intention is that the users of ROCm installation detector will not have to rewrite options handling code each time the bitcode libraries are queried. This is not too distant from detectors for other architecture that encapsulate the similar decision making process, providing cleaner interface. The only flag left in `getCommonBitcodeLibs` (main point of entry) is `NeedsASanRT`, this is deliberate, as in order to calculate it we need to consult `ToolChain`.
1 parent d385e9d commit 073460a

File tree

11 files changed

+136
-83
lines changed

11 files changed

+136
-83
lines changed

‎clang/include/clang/Driver/ToolChain.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,8 @@ class ToolChain {
802802

803803
/// Get paths for device libraries.
804804
virtual llvm::SmallVector<BitCodeLibraryInfo, 12>
805-
getDeviceLibs(const llvm::opt::ArgList &Args) const;
805+
getDeviceLibs(const llvm::opt::ArgList &Args,
806+
const Action::OffloadKind DeviceOffloadingKind) const;
806807

807808
/// Add the system specific linker arguments to use
808809
/// for the given HIP runtime library type.

‎clang/lib/Driver/ToolChain.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1631,7 +1631,8 @@ void ToolChain::addSYCLIncludeArgs(const ArgList &DriverArgs,
16311631
ArgStringList &CC1Args) const {}
16321632

16331633
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
1634-
ToolChain::getDeviceLibs(const ArgList &DriverArgs) const {
1634+
ToolChain::getDeviceLibs(const ArgList &DriverArgs,
1635+
const Action::OffloadKind DeviceOffloadingKind) const {
16351636
return {};
16361637
}
16371638

‎clang/lib/Driver/ToolChains/AMDGPU.cpp

Lines changed: 84 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,68 @@ using namespace clang::driver::toolchains;
3131
using namespace clang;
3232
using namespace llvm::opt;
3333

34+
RocmInstallationDetector::CommonBitcodeLibsPreferences::
35+
CommonBitcodeLibsPreferences(const Driver &D,
36+
const llvm::opt::ArgList &DriverArgs,
37+
StringRef GPUArch,
38+
const Action::OffloadKind DeviceOffloadingKind,
39+
const bool NeedsASanRT)
40+
: ABIVer(DeviceLibABIVersion::fromCodeObjectVersion(
41+
tools::getAMDGPUCodeObjectVersion(D, DriverArgs))) {
42+
const auto Kind = llvm::AMDGPU::parseArchAMDGCN(GPUArch);
43+
const unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(Kind);
44+
45+
IsOpenMP = DeviceOffloadingKind == Action::OFK_OpenMP;
46+
47+
const bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
48+
Wave64 =
49+
!HasWave32 || DriverArgs.hasFlag(options::OPT_mwavefrontsize64,
50+
options::OPT_mno_wavefrontsize64, false);
51+
52+
const bool IsKnownOffloading = DeviceOffloadingKind == Action::OFK_OpenMP ||
53+
DeviceOffloadingKind == Action::OFK_HIP;
54+
55+
// Default to enabling f32 denormals on subtargets where fma is fast with
56+
// denormals
57+
const bool DefaultDAZ =
58+
(Kind == llvm::AMDGPU::GK_NONE)
59+
? false
60+
: !((ArchAttr & llvm::AMDGPU::FEATURE_FAST_FMA_F32) &&
61+
(ArchAttr & llvm::AMDGPU::FEATURE_FAST_DENORMAL_F32));
62+
// TODO: There are way too many flags that change this. Do we need to
63+
// check them all?
64+
DAZ = IsKnownOffloading
65+
? DriverArgs.hasFlag(options::OPT_fgpu_flush_denormals_to_zero,
66+
options::OPT_fno_gpu_flush_denormals_to_zero,
67+
DefaultDAZ)
68+
: DriverArgs.hasArg(options::OPT_cl_denorms_are_zero) || DefaultDAZ;
69+
70+
FiniteOnly = DriverArgs.hasArg(options::OPT_cl_finite_math_only) ||
71+
DriverArgs.hasFlag(options::OPT_ffinite_math_only,
72+
options::OPT_fno_finite_math_only, false);
73+
74+
UnsafeMathOpt =
75+
DriverArgs.hasArg(options::OPT_cl_unsafe_math_optimizations) ||
76+
DriverArgs.hasFlag(options::OPT_funsafe_math_optimizations,
77+
options::OPT_fno_unsafe_math_optimizations, false);
78+
79+
FastRelaxedMath = DriverArgs.hasArg(options::OPT_cl_fast_relaxed_math) ||
80+
DriverArgs.hasFlag(options::OPT_ffast_math,
81+
options::OPT_fno_fast_math, false);
82+
83+
const bool DefaultSqrt = IsKnownOffloading ? true : false;
84+
CorrectSqrt =
85+
DriverArgs.hasArg(options::OPT_cl_fp32_correctly_rounded_divide_sqrt) ||
86+
DriverArgs.hasFlag(
87+
options::OPT_fhip_fp32_correctly_rounded_divide_sqrt,
88+
options::OPT_fno_hip_fp32_correctly_rounded_divide_sqrt, DefaultSqrt);
89+
// GPU Sanitizer currently only supports ASan and is enabled through host
90+
// ASan.
91+
GPUSan = (DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
92+
options::OPT_fno_gpu_sanitize, true) &&
93+
NeedsASanRT);
94+
}
95+
3496
void RocmInstallationDetector::scanLibDevicePath(llvm::StringRef Path) {
3597
assert(!Path.empty());
3698

@@ -884,33 +946,14 @@ void ROCMToolChain::addClangTargetOptions(
884946
ABIVer))
885947
return;
886948

887-
bool Wave64 = isWave64(DriverArgs, Kind);
888-
// TODO: There are way too many flags that change this. Do we need to check
889-
// them all?
890-
bool DAZ = DriverArgs.hasArg(options::OPT_cl_denorms_are_zero) ||
891-
getDefaultDenormsAreZeroForTarget(Kind);
892-
bool FiniteOnly = DriverArgs.hasArg(options::OPT_cl_finite_math_only);
893-
894-
bool UnsafeMathOpt =
895-
DriverArgs.hasArg(options::OPT_cl_unsafe_math_optimizations);
896-
bool FastRelaxedMath = DriverArgs.hasArg(options::OPT_cl_fast_relaxed_math);
897-
bool CorrectSqrt =
898-
DriverArgs.hasArg(options::OPT_cl_fp32_correctly_rounded_divide_sqrt);
899-
900-
// GPU Sanitizer currently only supports ASan and is enabled through host
901-
// ASan.
902-
bool GPUSan = DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
903-
options::OPT_fno_gpu_sanitize, true) &&
904-
getSanitizerArgs(DriverArgs).needsAsanRt();
905-
906949
// Add the OpenCL specific bitcode library.
907950
llvm::SmallVector<BitCodeLibraryInfo, 12> BCLibs;
908951
BCLibs.emplace_back(RocmInstallation->getOpenCLPath().str());
909952

910953
// Add the generic set of libraries.
911954
BCLibs.append(RocmInstallation->getCommonBitcodeLibs(
912-
DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt,
913-
FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, false));
955+
DriverArgs, LibDeviceFile, GpuArch, DeviceOffloadingKind,
956+
getSanitizerArgs(DriverArgs).needsAsanRt()));
914957

915958
for (auto [BCFile, Internalize] : BCLibs) {
916959
if (Internalize)
@@ -947,45 +990,47 @@ bool RocmInstallationDetector::checkCommonBitcodeLibs(
947990

948991
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
949992
RocmInstallationDetector::getCommonBitcodeLibs(
950-
const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile, bool Wave64,
951-
bool DAZ, bool FiniteOnly, bool UnsafeMathOpt, bool FastRelaxedMath,
952-
bool CorrectSqrt, DeviceLibABIVersion ABIVer, bool GPUSan,
953-
bool isOpenMP) const {
993+
const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile,
994+
StringRef GPUArch, const Action::OffloadKind DeviceOffloadingKind,
995+
const bool NeedsASanRT) const {
954996
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> BCLibs;
955997

998+
CommonBitcodeLibsPreferences Pref{D, DriverArgs, GPUArch,
999+
DeviceOffloadingKind, NeedsASanRT};
1000+
9561001
auto AddBCLib = [&](ToolChain::BitCodeLibraryInfo BCLib,
9571002
bool Internalize = true) {
9581003
BCLib.ShouldInternalize = Internalize;
9591004
BCLibs.emplace_back(BCLib);
9601005
};
9611006
auto AddSanBCLibs = [&]() {
962-
if (GPUSan)
1007+
if (Pref.GPUSan)
9631008
AddBCLib(getAsanRTLPath(), false);
9641009
};
9651010

9661011
AddSanBCLibs();
9671012
AddBCLib(getOCMLPath());
968-
if (!isOpenMP)
1013+
if (!Pref.IsOpenMP)
9691014
AddBCLib(getOCKLPath());
970-
else if (GPUSan && isOpenMP)
1015+
else if (Pref.GPUSan && Pref.IsOpenMP)
9711016
AddBCLib(getOCKLPath(), false);
972-
AddBCLib(getDenormalsAreZeroPath(DAZ));
973-
AddBCLib(getUnsafeMathPath(UnsafeMathOpt || FastRelaxedMath));
974-
AddBCLib(getFiniteOnlyPath(FiniteOnly || FastRelaxedMath));
975-
AddBCLib(getCorrectlyRoundedSqrtPath(CorrectSqrt));
976-
AddBCLib(getWavefrontSize64Path(Wave64));
1017+
AddBCLib(getDenormalsAreZeroPath(Pref.DAZ));
1018+
AddBCLib(getUnsafeMathPath(Pref.UnsafeMathOpt || Pref.FastRelaxedMath));
1019+
AddBCLib(getFiniteOnlyPath(Pref.FiniteOnly || Pref.FastRelaxedMath));
1020+
AddBCLib(getCorrectlyRoundedSqrtPath(Pref.CorrectSqrt));
1021+
AddBCLib(getWavefrontSize64Path(Pref.Wave64));
9771022
AddBCLib(LibDeviceFile);
978-
auto ABIVerPath = getABIVersionPath(ABIVer);
1023+
auto ABIVerPath = getABIVersionPath(Pref.ABIVer);
9791024
if (!ABIVerPath.empty())
9801025
AddBCLib(ABIVerPath);
9811026

9821027
return BCLibs;
9831028
}
9841029

9851030
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
986-
ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
987-
const std::string &GPUArch,
988-
bool isOpenMP) const {
1031+
ROCMToolChain::getCommonDeviceLibNames(
1032+
const llvm::opt::ArgList &DriverArgs, const std::string &GPUArch,
1033+
Action::OffloadKind DeviceOffloadingKind) const {
9891034
auto Kind = llvm::AMDGPU::parseArchAMDGCN(GPUArch);
9901035
const StringRef CanonArch = llvm::AMDGPU::getArchNameAMDGCN(Kind);
9911036

@@ -996,33 +1041,9 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
9961041
ABIVer))
9971042
return {};
9981043

999-
// If --hip-device-lib is not set, add the default bitcode libraries.
1000-
// TODO: There are way too many flags that change this. Do we need to check
1001-
// them all?
1002-
bool DAZ = DriverArgs.hasFlag(options::OPT_fgpu_flush_denormals_to_zero,
1003-
options::OPT_fno_gpu_flush_denormals_to_zero,
1004-
getDefaultDenormsAreZeroForTarget(Kind));
1005-
bool FiniteOnly = DriverArgs.hasFlag(
1006-
options::OPT_ffinite_math_only, options::OPT_fno_finite_math_only, false);
1007-
bool UnsafeMathOpt =
1008-
DriverArgs.hasFlag(options::OPT_funsafe_math_optimizations,
1009-
options::OPT_fno_unsafe_math_optimizations, false);
1010-
bool FastRelaxedMath = DriverArgs.hasFlag(options::OPT_ffast_math,
1011-
options::OPT_fno_fast_math, false);
1012-
bool CorrectSqrt = DriverArgs.hasFlag(
1013-
options::OPT_fhip_fp32_correctly_rounded_divide_sqrt,
1014-
options::OPT_fno_hip_fp32_correctly_rounded_divide_sqrt, true);
1015-
bool Wave64 = isWave64(DriverArgs, Kind);
1016-
1017-
// GPU Sanitizer currently only supports ASan and is enabled through host
1018-
// ASan.
1019-
bool GPUSan = DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
1020-
options::OPT_fno_gpu_sanitize, true) &&
1021-
getSanitizerArgs(DriverArgs).needsAsanRt();
1022-
10231044
return RocmInstallation->getCommonBitcodeLibs(
1024-
DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt,
1025-
FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, isOpenMP);
1045+
DriverArgs, LibDeviceFile, GPUArch, DeviceOffloadingKind,
1046+
getSanitizerArgs(DriverArgs).needsAsanRt());
10261047
}
10271048

10281049
bool AMDGPUToolChain::shouldSkipSanitizeOption(

‎clang/lib/Driver/ToolChains/AMDGPU.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
147147
llvm::SmallVector<BitCodeLibraryInfo, 12>
148148
getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
149149
const std::string &GPUArch,
150-
bool isOpenMP = false) const;
150+
Action::OffloadKind DeviceOffloadingKind) const;
151151

152152
SanitizerMask getSupportedSanitizers() const override {
153153
return SanitizerKind::Address;

‎clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
4444
true))
4545
return;
4646

47-
for (auto BCFile : getDeviceLibs(DriverArgs)) {
47+
for (auto BCFile : getDeviceLibs(DriverArgs, DeviceOffloadingKind)) {
4848
CC1Args.push_back(BCFile.ShouldInternalize ? "-mlink-builtin-bitcode"
4949
: "-mlink-bitcode-file");
5050
CC1Args.push_back(DriverArgs.MakeArgString(BCFile.Path));
@@ -132,16 +132,18 @@ AMDGPUOpenMPToolChain::computeMSVCVersion(const Driver *D,
132132
}
133133

134134
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
135-
AMDGPUOpenMPToolChain::getDeviceLibs(const llvm::opt::ArgList &Args) const {
135+
AMDGPUOpenMPToolChain::getDeviceLibs(
136+
const llvm::opt::ArgList &Args,
137+
const Action::OffloadKind DeviceOffloadingKind) const {
136138
if (!Args.hasFlag(options::OPT_offloadlib, options::OPT_no_offloadlib, true))
137139
return {};
138140

139141
StringRef GpuArch = getProcessorFromTargetID(
140142
getTriple(), Args.getLastArgValue(options::OPT_march_EQ));
141143

142144
SmallVector<BitCodeLibraryInfo, 12> BCLibs;
143-
for (auto BCLib : getCommonDeviceLibNames(Args, GpuArch.str(),
144-
/*IsOpenMP=*/true))
145+
for (auto BCLib :
146+
getCommonDeviceLibNames(Args, GpuArch.str(), DeviceOffloadingKind))
145147
BCLibs.emplace_back(BCLib);
146148

147149
return BCLibs;

‎clang/lib/Driver/ToolChains/AMDGPUOpenMP.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUOpenMPToolChain final
5858
const llvm::opt::ArgList &Args) const override;
5959

6060
llvm::SmallVector<BitCodeLibraryInfo, 12>
61-
getDeviceLibs(const llvm::opt::ArgList &Args) const override;
61+
getDeviceLibs(const llvm::opt::ArgList &Args,
62+
const Action::OffloadKind DeviceOffloadKind) const override;
6263

6364
const ToolChain &HostTC;
6465
};

‎clang/lib/Driver/ToolChains/HIPAMD.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ void HIPAMDToolChain::addClangTargetOptions(
264264
return; // No DeviceLibs for SPIR-V.
265265
}
266266

267-
for (auto BCFile : getDeviceLibs(DriverArgs)) {
267+
for (auto BCFile : getDeviceLibs(DriverArgs, DeviceOffloadingKind)) {
268268
CC1Args.push_back(BCFile.ShouldInternalize ? "-mlink-builtin-bitcode"
269269
: "-mlink-bitcode-file");
270270
CC1Args.push_back(DriverArgs.MakeArgString(BCFile.Path));
@@ -355,7 +355,8 @@ VersionTuple HIPAMDToolChain::computeMSVCVersion(const Driver *D,
355355
}
356356

357357
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
358-
HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
358+
HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs,
359+
Action::OffloadKind DeviceOffloadingKind) const {
359360
llvm::SmallVector<BitCodeLibraryInfo, 12> BCLibs;
360361
if (!DriverArgs.hasFlag(options::OPT_offloadlib, options::OPT_no_offloadlib,
361362
true) ||
@@ -397,7 +398,8 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
397398
assert(!GpuArch.empty() && "Must have an explicit GPU arch.");
398399

399400
// Add common device libraries like ocml etc.
400-
for (auto N : getCommonDeviceLibNames(DriverArgs, GpuArch.str()))
401+
for (auto N : getCommonDeviceLibNames(DriverArgs, GpuArch.str(),
402+
DeviceOffloadingKind))
401403
BCLibs.emplace_back(N);
402404

403405
// Add instrument lib.

‎clang/lib/Driver/ToolChains/HIPAMD.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ class LLVM_LIBRARY_VISIBILITY HIPAMDToolChain final : public ROCMToolChain {
8080
void AddHIPIncludeArgs(const llvm::opt::ArgList &DriverArgs,
8181
llvm::opt::ArgStringList &CC1Args) const override;
8282
llvm::SmallVector<BitCodeLibraryInfo, 12>
83-
getDeviceLibs(const llvm::opt::ArgList &Args) const override;
83+
getDeviceLibs(const llvm::opt::ArgList &Args,
84+
Action::OffloadKind DeviceOffloadKind) const override;
8485

8586
SanitizerMask getSupportedSanitizers() const override;
8687

‎clang/lib/Driver/ToolChains/HIPSPV.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,8 @@ void HIPSPVToolChain::addClangTargetOptions(
149149
CC1Args.append(
150150
{"-fvisibility=hidden", "-fapply-global-visibility-to-externs"});
151151

152-
for (const BitCodeLibraryInfo &BCFile : getDeviceLibs(DriverArgs))
152+
for (const BitCodeLibraryInfo &BCFile :
153+
getDeviceLibs(DriverArgs, DeviceOffloadingKind))
153154
CC1Args.append(
154155
{"-mlink-builtin-bitcode", DriverArgs.MakeArgString(BCFile.Path)});
155156
}
@@ -200,7 +201,9 @@ void HIPSPVToolChain::AddHIPIncludeArgs(const ArgList &DriverArgs,
200201
}
201202

202203
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
203-
HIPSPVToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
204+
HIPSPVToolChain::getDeviceLibs(
205+
const llvm::opt::ArgList &DriverArgs,
206+
const Action::OffloadKind DeviceOffloadingKind) const {
204207
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> BCLibs;
205208
if (!DriverArgs.hasFlag(options::OPT_offloadlib, options::OPT_no_offloadlib,
206209
true))

‎clang/lib/Driver/ToolChains/HIPSPV.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ class LLVM_LIBRARY_VISIBILITY HIPSPVToolChain final : public ToolChain {
6969
void AddHIPIncludeArgs(const llvm::opt::ArgList &DriverArgs,
7070
llvm::opt::ArgStringList &CC1Args) const override;
7171
llvm::SmallVector<BitCodeLibraryInfo, 12>
72-
getDeviceLibs(const llvm::opt::ArgList &Args) const override;
72+
getDeviceLibs(const llvm::opt::ArgList &Args,
73+
const Action::OffloadKind DeviceOffloadKind) const override;
7374

7475
SanitizerMask getSupportedSanitizers() const override;
7576

0 commit comments

Comments
 (0)