[clang][Driver] Add HIPAMD Driver support for AMDGCN flavoured SPIR-V #95061
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,6 +147,11 @@ 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)) { | ||
| if (Args.hasArgNoClaim(options::OPT_offload_arch_EQ)) { | ||
| if (Args.getLastArgValue(options::OPT_offload_arch_EQ) == "amdgcnspirv") | ||
| return llvm::Triple("spirv64-amd-amdhsa"); | ||
|
There was a problem hiding this comment. 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. 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. I am OK to commit this since the command line option won't change so users are not affected. |
||
| return llvm::Triple("amdgcn-amd-amdhsa"); | ||
| } | ||
| return llvm::Triple("amdgcn-amd-amdhsa"); // Default HIP triple. | ||
| } | ||
| auto TT = getOffloadTargetTriple(D, Args); | ||
|
|
@@ -3247,10 +3252,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()) { | ||
| 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; | ||
|
|
@@ -6516,9 +6525,11 @@ const ToolChain &Driver::getOffloadingDeviceToolChain( | |
| // things. | ||
| switch (TargetDeviceOffloadKind) { | ||
| case Action::OFK_HIP: { | ||
| 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 && | ||
|
|
||
This file contains 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
This file contains 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
| 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. @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. 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 || | ||
| 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) || | ||
| (getTriple().getArch() == llvm::Triple::spirv64 && | ||
| getTriple().getVendor() == llvm::Triple::AMD)) | ||
| return {}; | ||
| ArgStringList LibraryPaths; | ||
|
|
||
|
|
||
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
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.
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
isAMDtollvm::Tripleor 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?