-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[HIP][SPIRV] Enable the SPIRV backend instead of the translator through an experimental flag. #162282
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
base: main
Are you sure you want to change the base?
Conversation
1d30ec3
to
dfddf9d
Compare
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-backend-amdgpu Author: Manuel Carrasco (mgcarrasco) ChangesThis PR enables the experimental support for calling the SPIRV backend instead of the translator. Depends on #162096 and #162082. Full diff: https://github.com/llvm/llvm-project/pull/162282.diff 10 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 9bfa1dd52effe..7844bd4cb7f49 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5462,6 +5462,20 @@ defm wavefrontsize64 : SimpleMFlag<"wavefrontsize64",
defm amdgpu_precise_memory_op
: SimpleMFlag<"amdgpu-precise-memory-op", "Enable", "Disable",
" precise memory mode (AMDGPU only)">;
+def amdgpu_use_experimental_spirv_backend
+ : Flag<["-"], "amdgpu-use-experimental-spirv-backend">,
+ Group<m_amdgpu_Features_Group>,
+ Flags<[HelpHidden]>,
+ Visibility<[ClangOption]>,
+ HelpText<"Use experimental SPIRV backend for AMDGPU compilation (AMDGPU "
+ "only)">;
+def no_amdgpu_use_experimental_spirv_backend
+ : Flag<["-"], "no-amdgpu-use-experimental-spirv-backend">,
+ Group<m_amdgpu_Features_Group>,
+ Flags<[HelpHidden]>,
+ Visibility<[ClangOption]>,
+ HelpText<"Do not use experimental SPIRV backend for AMDGPU compilation "
+ "(AMDGPU only)">;
def munsafe_fp_atomics : Flag<["-"], "munsafe-fp-atomics">,
Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, Alias<fatomic_ignore_denormal_mode>;
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 85a1335785542..f05956d3b0966 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -995,7 +995,7 @@ inferOffloadToolchains(Compilation &C, Action::OffloadKind Kind) {
StringRef Triple;
if (ID == OffloadArch::AMDGCNSPIRV)
- Triple = "spirv64-amd-amdhsa";
+ Triple = "spirv64v1.6-amd-amdhsa";
else if (IsNVIDIAOffloadArch(ID))
Triple = C.getDefaultToolChain().getTriple().isArch64Bit()
? "nvptx64-nvidia-cuda"
diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp
index 5f3fbea40f162..9305f831609f0 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -168,19 +168,39 @@ void AMDGCN::Linker::constructLinkAndEmitSpirvCommand(
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-unknown-intrinsics",
- "--spirv-lower-const-expr",
- "--spirv-preserve-auxdata",
- "--spirv-debug-info-version=nonsemantic-shader-200"};
- SPIRV::constructTranslateCommand(C, *this, JA, Output, Output, TrArgs);
+ std::string LinkedBCFilePrefix(
+ Twine(llvm::sys::path::stem(Output.getFilename()), "-linked").str());
+ const char *LinkedBCFilePath = HIP::getTempFile(C, LinkedBCFilePrefix, "bc");
+ InputInfo LinkedBCFile(&JA, LinkedBCFilePath, Output.getBaseInput());
+
+ constructLlvmLinkCommand(C, JA, Inputs, LinkedBCFile, Args);
+
+ bool UseSPIRVBackend = Args.hasFlag(
+ options::OPT_amdgpu_use_experimental_spirv_backend,
+ options::OPT_no_amdgpu_use_experimental_spirv_backend, false);
+
+ if (UseSPIRVBackend) {
+ llvm::opt::ArgStringList CmdArgs;
+ CmdArgs.push_back(LinkedBCFile.getFilename());
+ CmdArgs.append({"-o", Output.getFilename()});
+ const char *Exec =
+ C.getArgs().MakeArgString(getToolChain().GetProgramPath("llc"));
+ CmdArgs.push_back("-mtriple=spirv64v1.6-amd-amdhsa");
+ C.addCommand(std::make_unique<Command>(JA, *this,
+ ResponseFileSupport::None(), Exec,
+ CmdArgs, LinkedBCFile, Output));
+ } else {
+ // Emit SPIR-V binary.
+ llvm::opt::ArgStringList TrArgs{
+ "--spirv-max-version=1.6",
+ "--spirv-ext=+all",
+ "--spirv-allow-unknown-intrinsics",
+ "--spirv-lower-const-expr",
+ "--spirv-preserve-auxdata",
+ "--spirv-debug-info-version=nonsemantic-shader-200"};
+ SPIRV::constructTranslateCommand(C, *this, JA, Output, LinkedBCFile,
+ TrArgs);
+ }
}
// For amdgcn the inputs of the linker job are device bitcode and output is
diff --git a/clang/lib/Driver/ToolChains/HIPSPV.cpp b/clang/lib/Driver/ToolChains/HIPSPV.cpp
index 62bca0493a09a..bce7f46dea468 100644
--- a/clang/lib/Driver/ToolChains/HIPSPV.cpp
+++ b/clang/lib/Driver/ToolChains/HIPSPV.cpp
@@ -22,17 +22,6 @@ using namespace clang::driver::tools;
using namespace clang;
using namespace llvm::opt;
-// Convenience function for creating temporary file for both modes of
-// isSaveTempsEnabled().
-static const char *getTempFile(Compilation &C, StringRef Prefix,
- StringRef Extension) {
- if (C.getDriver().isSaveTempsEnabled()) {
- return C.getArgs().MakeArgString(Prefix + "." + Extension);
- }
- auto TmpFile = C.getDriver().GetTemporaryPath(Prefix, Extension);
- return C.addTempFile(C.getArgs().MakeArgString(TmpFile));
-}
-
// Locates HIP pass plugin.
static std::string findPassPlugin(const Driver &D,
const llvm::opt::ArgList &Args) {
@@ -65,7 +54,7 @@ void HIPSPV::Linker::constructLinkAndEmitSpirvCommand(
assert(!Inputs.empty() && "Must have at least one input.");
std::string Name = std::string(llvm::sys::path::stem(Output.getFilename()));
- const char *TempFile = getTempFile(C, Name + "-link", "bc");
+ const char *TempFile = HIP::getTempFile(C, Name + "-link", "bc");
// Link LLVM bitcode.
ArgStringList LinkArgs{};
@@ -93,7 +82,7 @@ void HIPSPV::Linker::constructLinkAndEmitSpirvCommand(
auto PassPluginPath = findPassPlugin(C.getDriver(), Args);
if (!PassPluginPath.empty()) {
const char *PassPathCStr = C.getArgs().MakeArgString(PassPluginPath);
- const char *OptOutput = getTempFile(C, Name + "-lower", "bc");
+ const char *OptOutput = HIP::getTempFile(C, Name + "-lower", "bc");
ArgStringList OptArgs{TempFile, "-load-pass-plugin",
PassPathCStr, "-passes=hip-post-link-passes",
"-o", OptOutput};
diff --git a/clang/lib/Driver/ToolChains/HIPUtility.cpp b/clang/lib/Driver/ToolChains/HIPUtility.cpp
index cb061ffede234..732403e69a075 100644
--- a/clang/lib/Driver/ToolChains/HIPUtility.cpp
+++ b/clang/lib/Driver/ToolChains/HIPUtility.cpp
@@ -472,3 +472,14 @@ void HIP::constructGenerateObjFileFromHIPFatBinary(
D.getClangProgramPath(), ClangArgs,
Inputs, Output, D.getPrependArg()));
}
+
+// Convenience function for creating temporary file for both modes of
+// isSaveTempsEnabled().
+const char *HIP::getTempFile(Compilation &C, StringRef Prefix,
+ StringRef Extension) {
+ if (C.getDriver().isSaveTempsEnabled()) {
+ return C.getArgs().MakeArgString(Prefix + "." + Extension);
+ }
+ auto TmpFile = C.getDriver().GetTemporaryPath(Prefix, Extension);
+ return C.addTempFile(C.getArgs().MakeArgString(TmpFile));
+}
diff --git a/clang/lib/Driver/ToolChains/HIPUtility.h b/clang/lib/Driver/ToolChains/HIPUtility.h
index 29e5a922024ab..55c155e5c35cf 100644
--- a/clang/lib/Driver/ToolChains/HIPUtility.h
+++ b/clang/lib/Driver/ToolChains/HIPUtility.h
@@ -16,6 +16,8 @@ namespace driver {
namespace tools {
namespace HIP {
+const char *getTempFile(Compilation &C, StringRef Prefix, StringRef Extension);
+
// Construct command for creating HIP fatbin.
void constructHIPFatbinCommand(Compilation &C, const JobAction &JA,
StringRef OutputFileName,
diff --git a/clang/test/Driver/amdgpu-spirv-backend-opt.c b/clang/test/Driver/amdgpu-spirv-backend-opt.c
new file mode 100644
index 0000000000000..d866ce910b70d
--- /dev/null
+++ b/clang/test/Driver/amdgpu-spirv-backend-opt.c
@@ -0,0 +1,12 @@
+// COM: This test case validates the behavior of -amdgpu-use-experimental-spirv-backend
+
+// COM: Test that -amdgpu-use-experimental-spirv-backend calls llc with the SPIRV triple.
+// RUN: %clang -x hip %s --cuda-device-only --offload-arch=amdgcnspirv -amdgpu-use-experimental-spirv-backend -### 2>&1 | FileCheck %s --check-prefix=CHECK-SPIRV-BACKEND
+// CHECK-SPIRV-BACKEND: "{{.*}}llc{{.*}}" "{{-mtriple=spirv64v[0-9]+\.[0-9]+-amd-amdhsa}}"
+
+// COM: Test that -no-amdgpu-use-experimental-spirv-backend calls the SPIRV translator
+// COM: %clang -x hip %s --cuda-device-only --offload-arch=amdgcnspirv -no-amdgpu-use-experimental-spirv-backend -### 2>&1 | FileCheck %s --check-prefix=CHECK-SPIRV-TRANSLATOR
+// CHECK-SPIRV-TRANSLATOR: "{{.*llvm-spirv.*}}" "{{--spirv-max-version=[0-9]+\.[0-9]}}"
+
+// COM: Test that by default we use the translator
+// RUN: %clang -x hip %s --cuda-device-only --offload-arch=amdgcnspirv -### 2>&1 | FileCheck %s --check-prefix=CHECK-SPIRV-TRANSLATOR
\ No newline at end of file
diff --git a/clang/test/Driver/hip-phases.hip b/clang/test/Driver/hip-phases.hip
index 6bac97ab8064b..f50a7fb1a2a86 100644
--- a/clang/test/Driver/hip-phases.hip
+++ b/clang/test/Driver/hip-phases.hip
@@ -700,6 +700,6 @@
// SPIRV-ONLY-NEXT: 9: compiler, {8}, ir, (device-hip, amdgcnspirv)
// SPIRV-ONLY-NEXT: 10: backend, {9}, ir, (device-hip, amdgcnspirv)
// SPIRV-ONLY-NEXT: 11: linker, {10}, image, (device-hip, amdgcnspirv)
-// SPIRV-ONLY-NEXT: 12: offload, "device-hip (spirv64-amd-amdhsa:amdgcnspirv)" {11}, image
+// SPIRV-ONLY-NEXT: 12: offload, "device-hip (spirv64v{{[0-9]+\.[0-9]+}}-amd-amdhsa:amdgcnspirv)" {11}, image
// SPIRV-ONLY-NEXT: 13: linker, {6, 12}, hip-fatbin, (device-hip)
// SPIRV-ONLY-NEXT: 14: offload, "device-hip (amdgcn-amd-amdhsa)" {13}, none
diff --git a/clang/test/Driver/hip-toolchain-no-rdc.hip b/clang/test/Driver/hip-toolchain-no-rdc.hip
index dc8f0a97ad371..a9e7de9aa0040 100644
--- a/clang/test/Driver/hip-toolchain-no-rdc.hip
+++ b/clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -207,7 +207,7 @@
//
// AMDGCNSPIRV: "-cc1" "-triple" "spirv64-amd-amdhsa" {{.*}}"-emit-llvm-bc" {{.*}}"-fembed-bitcode=marker" "-disable-llvm-passes" {{.*}} "-o" "[[AMDGCNSPV_BC:.*bc]]"
-// AMDGCNSPIRV: {{".*llvm-link.*"}} "-o" "[[AMDGCNSPV_TMP:.*out]]" "[[AMDGCNSPV_BC]]"
+// AMDGCNSPIRV: {{".*llvm-link.*"}} "-o" "[[AMDGCNSPV_TMP:.*bc]]" "[[AMDGCNSPV_BC]]"
// AMDGCNSPIRV: {{".*llvm-spirv.*"}} "--spirv-max-version=1.6" "--spirv-ext=+all" {{.*}} "[[AMDGCNSPV_TMP]]" {{.*}}"-o" "[[AMDGCNSPV_CO:.*out]]"
// AMDGCNSPIRV: "-cc1" "-triple" "amdgcn-amd-amdhsa" {{.*}}"-emit-obj" {{.*}}"-target-cpu" "gfx900"{{.*}} "-o" "[[GFX900_OBJ:.*o]]"
// AMDGCNSPIRV: {{".*lld.*"}} {{.*}}"-plugin-opt=mcpu=gfx900" {{.*}} "-o" "[[GFX900_CO:.*out]]" {{.*}}"[[GFX900_OBJ]]"
diff --git a/clang/test/Driver/spirv-amd-toolchain.c b/clang/test/Driver/spirv-amd-toolchain.c
index 14ba8f4632477..8f1f0f33e53f9 100644
--- a/clang/test/Driver/spirv-amd-toolchain.c
+++ b/clang/test/Driver/spirv-amd-toolchain.c
@@ -15,5 +15,5 @@
// RUN: %clang -### --target=spirv64-amd-amdhsa %s -nogpulib -nogpuinc 2>&1 \
// RUN: | FileCheck %s --check-prefix=INVOCATION
// INVOCATION: "-cc1" "-triple" "spirv64-amd-amdhsa" {{.*}}"-disable-llvm-optzns" {{.*}} "-o" "[[OUTPUT:.+]]" "-x" "c"
-// INVOCATION: "{{.*}}llvm-link" "-o" "a.out" "[[OUTPUT]]"
-// INVOCATION: "{{.*}}llvm-spirv" "--spirv-max-version=1.6" "--spirv-ext=+all" "--spirv-allow-unknown-intrinsics" "--spirv-lower-const-expr" "--spirv-preserve-auxdata" "--spirv-debug-info-version=nonsemantic-shader-200" "a.out" "-o" "a.out"
+// INVOCATION: "{{.*}}llvm-link" "-o" "[[LINKED_OUTPUT:.+]]" "[[OUTPUT]]"
+// INVOCATION: "{{.*}}llvm-spirv" "--spirv-max-version=1.6" "--spirv-ext=+all" "--spirv-allow-unknown-intrinsics" "--spirv-lower-const-expr" "--spirv-preserve-auxdata" "--spirv-debug-info-version=nonsemantic-shader-200" "[[LINKED_OUTPUT]]" "-o" "a.out"
|
…gh an experimental flag. Co-authored-by: Juan Manuel Martinez Caamaño <[email protected]>
f65ed81
to
e89ce89
Compare
Rebased. The PR no longer has dependencies on other pull requests. |
HelpText<"Compile HIP source to relocatable">; | ||
def fno_hip_emit_relocatable : Flag<["-"], "fno-hip-emit-relocatable">, | ||
HelpText<"Do not override toolchain to compile HIP source to relocatable">; | ||
def use_experimental_spirv_backend |
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.
There should be a defm
helper for no
variants right?
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.
Ty. If we don't require a member variable, then defm
does not seem to be the right option (see how gpu_bundle_output
and no_gpu_bundle_output
are defined).
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.
Accidentally had this as request changes.
…print-bindings test cases.
Updated PR so clang emits SPIRV directly when using the SPIRV backend and avoids the previous extra call. |
TYPE("ada", Ada, INVALID, nullptr, phases::Compile, phases::Backend, phases::Assemble, phases::Link) | ||
TYPE("assembler", PP_Asm, INVALID, "s", phases::Assemble, phases::Link) | ||
TYPE("assembler-with-cpp", Asm, PP_Asm, "S", phases::Preprocess, phases::Assemble, phases::Link) | ||
TYPE("spv", SPV, INVALID, "spv", phases::Backend) |
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.
SPIR-V in this context is just the assembler face as far as I'm aware, I think there's some magic setting that splits it if you need to. That's what CUDA does w/ its PTX target. I'd imagine this would imply clang consuming SPIR-V to give you back an ELF or LLVM-IR.
ActionList AL; | ||
Action *BackendAction = nullptr; | ||
bool AssembleAndLink = true; | ||
if (ToolChains.front()->getTriple().isSPIRV() || |
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.
SPIR-V here should be similar to what CUDA does, I would just expect to skip this step entirely. I'm fairly certain the normal pipeline works for SPIR-V since the standalone clang target does what I expect. The linker is probably the only bit you'd skip, the handling there is a little weird since CUDA overloads it to mean fatbinary
while HIP uses the offload bundler (for now).
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.
@jhuber6 thanks for the feedback! I'm not sure if I follow what's the problem or suggestion. What step(s) and for what scenario(s) can be skipped? If so, where that should/would be handled?
A backend job was always created independently of SPIRV prior to the PR. Now it has been updated so it does using the SPIRV backend directly if needed. The assembler and linker steps are now skipped for SPIRV when using the backend. When using the translator, we still have to maintain the previous phases.
@@ -0,0 +1,4 @@ | |||
// RUN: %clang -x hip %s --cuda-device-only --offload-arch=amdgcnspirv -use-experimental-spirv-backend -nogpuinc -nogpulib -ccc-print-bindings 2>&1 | FileCheck %s |
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.
Needs a test for the new driver, because if all goes well I'm going to completely delete the old offload driver next release.
This PR enables the experimental support for calling the SPIRV backend instead of the translator.