-
Notifications
You must be signed in to change notification settings - Fork 12.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang][Driver] Add HIPAMD Driver support for AMDGCN flavoured SPIR-V #95061
Changes from all commits
662f160
393ce66
2a10ad0
98db8f7
c359e0a
e98f3f5
c41726d
4698b58
aa1cd7c
f9729ef
8257cb1
900cd69
3307f17
eee6063
d2f4244
4cb4026
84a621d
1841385
0ce2da3
120b73c
f3942bd
31ac77d
0e9b1a1
83cd5e0
e9158b0
05074e7
e1fb93f
36c4bf6
5ffa186
cf1880c
4d85a1b
516e14c
bdc3eb5
361d47b
b088c72
e85b557
1d41787
8bcf2b2
9b3275b
5b764ec
ba3fb6f
3719c3b
7ca1c87
f3e5145
089bb9b
b60e753
8741e7c
5f775b8
4aa60c7
1c2d0fa
e8a78a2
294cd9c
f712c53
fe927e0
5ce5497
2220d37
83ec34c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -147,6 +147,14 @@ getNVIDIAOffloadTargetTriple(const Driver &D, const ArgList &Args, | |
static std::optional<llvm::Triple> | ||
getHIPOffloadTargetTriple(const Driver &D, const ArgList &Args) { | ||
if (!Args.hasArg(options::OPT_offload_EQ)) { | ||
auto OffloadArchs = Args.getAllArgValues(options::OPT_offload_arch_EQ); | ||
if (llvm::find(OffloadArchs, "amdgcnspirv") != OffloadArchs.cend()) { | ||
if (OffloadArchs.size() == 1) | ||
return llvm::Triple("spirv64-amd-amdhsa"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a toolchain with spirv64 as triple will cause trouble for us to support mixed amdgcn and spirv fat binaries, which is critical for us. Better to take the approach similar to #75357, i.e. treat spirv as a processor of amgcn triple, so that we can use HIPAMD toolchain for both spirv and real amdgcn processor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the pointer, much appreciated! I will revisit this/take your suggestion, but for the initial experimental support it was deemed acceptable to carry this temporary limitation (unless you strongly object, of course). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am OK to commit this since the command line option won't change so users are not affected. |
||
// Mixing specific & SPIR-V compilation is not supported for now. | ||
D.Diag(diag::err_drv_only_one_offload_target_supported); | ||
return std::nullopt; | ||
} | ||
return llvm::Triple("amdgcn-amd-amdhsa"); // Default HIP triple. | ||
} | ||
auto TT = getOffloadTargetTriple(D, Args); | ||
|
@@ -3247,10 +3255,14 @@ class OffloadingActionBuilder final { | |
// supported GPUs. sm_20 code should work correctly, if | ||
// suboptimally, on all newer GPUs. | ||
if (GpuArchList.empty()) { | ||
if (ToolChains.front()->getTriple().isSPIRV()) | ||
GpuArchList.push_back(CudaArch::Generic); | ||
else | ||
if (ToolChains.front()->getTriple().isSPIRV()) { | ||
if (ToolChains.front()->getTriple().getVendor() == llvm::Triple::AMD) | ||
GpuArchList.push_back(CudaArch::AMDGCNSPIRV); | ||
else | ||
GpuArchList.push_back(CudaArch::Generic); | ||
} else { | ||
GpuArchList.push_back(DefaultCudaArch); | ||
} | ||
} | ||
|
||
return Error; | ||
|
@@ -6517,9 +6529,11 @@ const ToolChain &Driver::getOffloadingDeviceToolChain( | |
// things. | ||
switch (TargetDeviceOffloadKind) { | ||
case Action::OFK_HIP: { | ||
if (Target.getArch() == llvm::Triple::amdgcn && | ||
Target.getVendor() == llvm::Triple::AMD && | ||
Target.getOS() == llvm::Triple::AMDHSA) | ||
if (((Target.getArch() == llvm::Triple::amdgcn || | ||
Target.getArch() == llvm::Triple::spirv64) && | ||
Target.getVendor() == llvm::Triple::AMD && | ||
Target.getOS() == llvm::Triple::AMDHSA) || | ||
!Args.hasArgNoClaim(options::OPT_offload_EQ)) | ||
TC = std::make_unique<toolchains::HIPAMDToolChain>(*this, Target, | ||
HostTC, Args); | ||
else if (Target.getArch() == llvm::Triple::spirv64 && | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
#include "AMDGPU.h" | ||
#include "CommonArgs.h" | ||
#include "HIPUtility.h" | ||
#include "SPIRV.h" | ||
#include "clang/Basic/Cuda.h" | ||
#include "clang/Basic/TargetID.h" | ||
#include "clang/Driver/Compilation.h" | ||
|
@@ -193,6 +194,33 @@ void AMDGCN::Linker::constructLldCommand(Compilation &C, const JobAction &JA, | |
Lld, LldArgs, Inputs, Output)); | ||
} | ||
|
||
// For SPIR-V the inputs for the job are device AMDGCN SPIR-V flavoured bitcode | ||
// and the output is either a compiled SPIR-V binary or bitcode (-emit-llvm). It | ||
// calls llvm-link and then the llvm-spirv translator. Once the SPIR-V BE will | ||
// be promoted from experimental, we will switch to using that. TODO: consider | ||
// if we want to run any targeted optimisations over IR here, over generic | ||
// SPIR-V. | ||
void AMDGCN::Linker::constructLinkAndEmitSpirvCommand( | ||
Compilation &C, const JobAction &JA, const InputInfoList &Inputs, | ||
const InputInfo &Output, const llvm::opt::ArgList &Args) const { | ||
assert(!Inputs.empty() && "Must have at least one input."); | ||
|
||
constructLlvmLinkCommand(C, JA, Inputs, Output, Args); | ||
|
||
// Linked BC is now in Output | ||
|
||
// Emit SPIR-V binary. | ||
llvm::opt::ArgStringList TrArgs{ | ||
"--spirv-max-version=1.6", | ||
"--spirv-ext=+all", | ||
"--spirv-allow-extra-diexpressions", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AlexVlx nit: if generation of NonSemantic.Shader.DebugInfo.200 is turned on - this option is not needed as the extended instruction already adds all DWARF expressions (including LLVM-specific expressions). |
||
"--spirv-allow-unknown-intrinsics", | ||
"--spirv-lower-const-expr", | ||
"--spirv-preserve-auxdata", | ||
"--spirv-debug-info-version=nonsemantic-shader-200"}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recently I've found this patch in gitlog and was intrigued, does this line mean, that AMD driver supports KhronosGroup/SPIRV-Registry#186 ? Just for my curiosity. It may also make me push the instruction set for ratification sooner :) |
||
SPIRV::constructTranslateCommand(C, *this, JA, Output, Output, TrArgs); | ||
} | ||
|
||
// For amdgcn the inputs of the linker job are device bitcode and output is | ||
// either an object file or bitcode (-emit-llvm). It calls llvm-link, opt, | ||
// llc, then lld steps. | ||
|
@@ -214,6 +242,9 @@ void AMDGCN::Linker::ConstructJob(Compilation &C, const JobAction &JA, | |
if (JA.getType() == types::TY_LLVM_BC) | ||
return constructLlvmLinkCommand(C, JA, Inputs, Output, Args); | ||
|
||
if (getToolChain().getTriple().isSPIRV()) | ||
return constructLinkAndEmitSpirvCommand(C, JA, Inputs, Output, Args); | ||
|
||
return constructLldCommand(C, JA, Inputs, Output, Args); | ||
} | ||
|
||
|
@@ -270,6 +301,13 @@ void HIPAMDToolChain::addClangTargetOptions( | |
CC1Args.push_back("-fapply-global-visibility-to-externs"); | ||
} | ||
|
||
// For SPIR-V we embed the command-line into the generated binary, in order to | ||
// retrieve it at JIT time and be able to do target specific compilation with | ||
// options that match the user-supplied ones. | ||
if (getTriple().isSPIRV() && | ||
!DriverArgs.hasArg(options::OPT_fembed_bitcode_marker)) | ||
CC1Args.push_back("-fembed-bitcode=marker"); | ||
|
||
for (auto BCFile : getDeviceLibs(DriverArgs)) { | ||
CC1Args.push_back(BCFile.ShouldInternalize ? "-mlink-builtin-bitcode" | ||
: "-mlink-bitcode-file"); | ||
|
@@ -303,7 +341,8 @@ HIPAMDToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args, | |
} | ||
|
||
Tool *HIPAMDToolChain::buildLinker() const { | ||
assert(getTriple().getArch() == llvm::Triple::amdgcn); | ||
assert(getTriple().getArch() == llvm::Triple::amdgcn || | ||
getTriple().getArch() == llvm::Triple::spirv64); | ||
return new tools::AMDGCN::Linker(*this); | ||
} | ||
|
||
|
@@ -358,7 +397,9 @@ VersionTuple HIPAMDToolChain::computeMSVCVersion(const Driver *D, | |
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> | ||
HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const { | ||
llvm::SmallVector<BitCodeLibraryInfo, 12> BCLibs; | ||
if (DriverArgs.hasArg(options::OPT_nogpulib)) | ||
if (DriverArgs.hasArg(options::OPT_nogpulib) || | ||
(getTriple().getArch() == llvm::Triple::spirv64 && | ||
getTriple().getVendor() == llvm::Triple::AMD)) | ||
return {}; | ||
ArgStringList LibraryPaths; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should add
isAMD
tollvm::Triple
or something.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know the vendor got used for anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have
isAMDGCN
, but that reflects the Arch. I guess it might be convenient sugar to have a predicate on vendors as well? I don't find the manual check excessively offensive FWIW.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This matches how we've documented it when we added the AMDGCN flavoured SPIR-V, and seemed to reflect the idea that this is SPIRV(64) with customisations for AMD as the vendor; do you think it is or can become problematic?