-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
[SPIR-V] Add SPIRV to LLVM_ALL_TARGETS #119653
[SPIR-V] Add SPIRV to LLVM_ALL_TARGETS #119653
Conversation
@llvm/pr-subscribers-github-workflow @llvm/pr-subscribers-backend-spir-v Author: Michal Paszkowski (michalpaszkowski) ChangesThis commit promotes the SPIR-V backend from experimental to official status. As a result, SPIR-V will be built by default, simplifying integration and increasing accessibility for downstream projects. Discussion and RFC on Discourse: https://discourse.llvm.org/t/rfc-promoting-spir-v-to-an-official-target/83614 Full diff: https://github.com/llvm/llvm-project/pull/119653.diff 3 Files Affected:
diff --git a/.github/workflows/spirv-tests.yml b/.github/workflows/spirv-tests.yml
index 75918e73e89737..2a47690fe79aec 100644
--- a/.github/workflows/spirv-tests.yml
+++ b/.github/workflows/spirv-tests.yml
@@ -25,5 +25,5 @@ jobs:
with:
build_target: check-llvm-codegen-spirv
projects:
- extra_cmake_args: '-DLLVM_TARGETS_TO_BUILD="" -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="SPIRV" -DLLVM_INCLUDE_SPIRV_TOOLS_TESTS=ON'
+ extra_cmake_args: '-DLLVM_TARGETS_TO_BUILD="SPIRV" -DLLVM_INCLUDE_SPIRV_TOOLS_TESTS=ON'
os_list: '["ubuntu-latest"]'
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index f14065ab037990..ad12100fdb5b89 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -486,6 +486,7 @@ set(LLVM_ALL_TARGETS
PowerPC
RISCV
Sparc
+ SPIRV
SystemZ
VE
WebAssembly
@@ -498,7 +499,6 @@ set(LLVM_ALL_EXPERIMENTAL_TARGETS
CSKY
DirectX
M68k
- SPIRV
Xtensa
)
diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md
index a5805e050bfdbe..07b6416e069645 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -47,6 +47,12 @@ for adding a new subsection. -->
same semantics. The normalizer makes it easier to spot semantic differences
when diffing two modules which have undergone different passes.
+* The SPIR-V backend is now an official LLVM target, enabling support for
+ SPIR-V across a range of APIs and use cases, including OpenCL, Vulkan, SYCL,
+ GLSL, and HLSL. This backend serves as a unified solution for diverse compute
+ and graphics workloads, offering a robust alternative to the Khronos SPIR-V
+ LLVM Translator.
+
* ...
<!-- If you would like to document a larger change, then you can add a
|
Leaving here the same comment I left on the forum: I support this move, given the back-end has been fulfilling the criteria for years now. It would be good to merge this before the next release branch, so that production tools that rely on it could pick the next version easily. So, LGTM, but I'll wait to approve, to let others have time to digest this. |
c4ebbc5
to
b9c63d0
Compare
Hello, sorry to butt in, but although OpenCL flavoured SPIR-V backend is pretty stable and robust at this point, Vulkan flavoured SPIR-V backend is definitely not, as was said above, and there is quite a big difference between the two. Thanks to @Keenuts there was a lot of progress on it, but there is still a lot of work to do, and it is still rapidly changing, it is still nowhere near usable at this point. This should be reflected in the documentation of the backend at least. It is not a blocker and can be done in a separate patch, just want to point this out. Maybe Vulkan support could be gated under a separate compilation flag, but probably doesn't matter much. |
Thanks for the input!
Yes, that's correct. As of today, there is still no way to generate useful vulkan shaders since HLSL resources lowering is still being worked on.
Agreeing on this, the fact that a part of the SPIR-V support is still not completely stable should be mentioned in the docs.
Do you mean gating the VK related triple use behind a compile flag, or the actual Vulkan files/parts of the backend? |
Yes, I meant either of those. Gating it at the tools level would be simpler, you can add a special flag that enables it just for clang and llc for example. Thinking about it now a simple warning when you pass vulkan triples would be ok too. |
It looks like the SPIRV target currently lists @iliya-diyachkov as the maintainer (see https://github.com/llvm/llvm-project/blob/main/llvm/Maintainers.md#spirv-backend). However, I think they haven't been very involved in the past year (at least going by commit history). It would be great to update this to one or more people who are more actively involved right now. There should be plenty of candidates for this target, but you'll probably know better than I who to nominate here. Edit: After a closer look, the commit history may be a bit misleading, as Ilia still occasionally does SPIRV code reviews, so maybe this information is less outdated than I initially thought. Would still be good to review :) |
b9c63d0
to
06f3baa
Compare
Happy New Year! Thanks to everyone for all the comments!
@nikic Thank you for reminding me about this! I asked @VyacheslavLevytskyy and @Keenuts to be maintainers. Slava is actively working on SYCL/DPC++ features and Nathan is focused on Vulkan/HLSL related changes (especially structurizer). |
This commit promotes the SPIR-V backend from experimental to official status. As a result, SPIR-V will be built by default, simplifying integration and increasing accessibility for downstream projects. Discussion and RFC on Discourse: https://discourse.llvm.org/t/rfc-promoting-spir-v-to-an-official-target/83614
06f3baa
to
6c2415a
Compare
Thank you to everyone who has reviewed this pull request, other patches, and contributed to the SPIR-V backend in the last two years! This is a big milestone! Please note that the failures in the SPIR-V Tests GitHub action are due to a known bug in spirv-val. This issue is being tracked in KhronosGroup/SPIRV-Tools#5407. To prevent similar issues in the future, I will make changes to the GitHub action so that it references a specific revision of SPIR-V Tools. Updates to this reference will be done manually when the spec changes or validator improves, so that such disruptions don't affect other contributors to the SPIR-V backend. If there are no new comments or concerns, I plan to merge this pull request by the end of this week. Please feel free to share any additional feedback. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/134/builds/11986 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/108/builds/8309 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/110/builds/3470 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/5791 Here is the relevant piece of the build log for the reference
|
Reverting the change temporarily (#123532) to fix the build failures reported above. I will create separate PRs to address each issue and recommit this change again. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/7558 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/24/builds/4380 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/25/builds/5778 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/3903 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/85/builds/4532 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/5367 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/2116 Here is the relevant piece of the build log for the reference
|
Adding SPIRV to LLVM_ALL_TARGETS (#119653) revealed a series of minor compilation problems and sanitizer complaints. This PR is to address the problem.
Adding SPIRV to LLVM_ALL_TARGETS (llvm/llvm-project#119653) revealed a series of minor compilation problems and sanitizer complaints. This PR is to address the problem.
Two PRs fixing the issues discovered by buildbots were merged:
New PR reapplying the patch: #123654 |
This commit promotes the SPIR-V backend from experimental to official status. As a result, SPIR-V will be built by default, simplifying integration and increasing accessibility for downstream projects. Discussion and RFC on Discourse: https://discourse.llvm.org/t/rfc-promoting-spir-v-to-an-official-target/83614 The PR reapplies the original patch #119653, reverted due to buildbot failures.
This commit promotes the SPIR-V backend from experimental to official status. As a result, SPIR-V will be built by default, simplifying integration and increasing accessibility for downstream projects. Discussion and RFC on Discourse: https://discourse.llvm.org/t/rfc-promoting-spir-v-to-an-official-target/83614 The PR reapplies the original patch llvm/llvm-project#119653, reverted due to buildbot failures.
Three more PRs fixing the issues discovered by buildbots:
New PR reapplying the patch: #123733 |
…est case and fix a memory leak (#123725) Adding SPIRV to LLVM_ALL_TARGETS (#119653) revealed a series of minor compilation problems and sanitizer complaints. This PR is to move unit tests resources (a Module ptr) from the class-scope to a local scope of the class member function to be sure that before the test env is teared down the ptr is released.
This commit promotes the SPIR-V backend from experimental to official status. As a result, SPIR-V will be built by default, simplifying integration and increasing accessibility for downstream projects. Discussion and RFC on Discourse: https://discourse.llvm.org/t/rfc-promoting-spir-v-to-an-official-target/83614 The PR reapplies the original patch #119653 and consecutive #123654, reverted due to buildbot failures.
…t. a unit test case and fix a memory leak (#123725) Adding SPIRV to LLVM_ALL_TARGETS (llvm/llvm-project#119653) revealed a series of minor compilation problems and sanitizer complaints. This PR is to move unit tests resources (a Module ptr) from the class-scope to a local scope of the class member function to be sure that before the test env is teared down the ptr is released.
This commit promotes the SPIR-V backend from experimental to official status. As a result, SPIR-V will be built by default, simplifying integration and increasing accessibility for downstream projects. Discussion and RFC on Discourse: https://discourse.llvm.org/t/rfc-promoting-spir-v-to-an-official-target/83614 The PR reapplies the original patch llvm/llvm-project#119653 and consecutive llvm/llvm-project#123654, reverted due to buildbot failures.
Adding SPIRV to LLVM_ALL_TARGETS (#119653) revealed a series of minor compilation problems and sanitizer complaints. This PR is to fix debug-type-pointer.ll test case.
Adding SPIRV to LLVM_ALL_TARGETS (llvm/llvm-project#119653) revealed a series of minor compilation problems and sanitizer complaints. This PR is to fix debug-type-pointer.ll test case.
This commit promotes the SPIR-V backend from experimental to official status. As a result, SPIR-V will be built by default, simplifying integration and increasing accessibility for downstream projects.
Discussion and RFC on Discourse: https://discourse.llvm.org/t/rfc-promoting-spir-v-to-an-official-target/83614