-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[HIP][SPIRV] Enable the SPIRV backend instead of the translator through an experimental flag (old driver). #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
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:
|
…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).
AlexVlx
left a comment
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.