Reland "[spirv] Switch to use common target description" #17699
Merged
Conversation
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 relands iree-org#17623. This commit switches SPIR-V side to use the common `#iree_gpu.target` to describe the GPU characteristics. With it we can now remove the ad-hoc Vulkan attributes and dialects and unify how GPU are described across various GPU compiler backends in IREE. SPIR-V has some additional requirements that we need to account for: We have many vendors and APIs to handle there so this commit adds various AMD/ARM/NVIDIA/Qualcomm targets for development purposes so that we can specify them with a shorthand. In order to be extensible, leverage the `feature` field in `#iree_gpu.target` to specify additional capabilities with `cap:` prefix and extensions with `ext:` prefix. We also use the `feature` field to specify what SPIR-V version to target with the `spirv:v1.x` format. Right now the `SPIRVConvertGPUTarget` pass is invoked immediately before configuration. This is to stage the changes. As a next step we need to move it immediately before `ConvertToSPIRV` pass. `--iree-vulkan-target-env` is dropped given now we removed the whole Vulkan dialect and cannot control with a `#vk.target_env` attribute anymore. The default `--iree-vulkan-target-triple` now becomes `vp_android_baseline_2022`, which is a a good lowest common denominator to guarantee the generated SPIR-V is widely accepted. We are not considering SwiftShader now anymore like previously due to testing purposes. The `--iree-vulkan-target-triple` should be renamed given it's not a triple anymore--that will happen later together with other GPU backends (i.e., cuda/hip) to be consistent. In order to support cooperative matrix conversion, we added `WMMA_F16_16x16x16_F16`. For NVIDIA GPUs we are abusing it right now without considering the concrete explicit layout--that is fine given in Vulkan they are opaque anyway. But this need to be fixed if we are targeting WMMA in CUDA. We now contruct a `#iree_gpu.target` to specify the target to drive SPIR-V CodeGen. Progress towards iree-org#16341 ci-extra: test_nvidia_gpu,test_nvidia_a100,test_amd_mi250, build_test_all_macos_arm64,build_and_test_android Signed-off-by: Lei Zhang <antiagainst@gmail.com>
Signed-off-by: Lei Zhang <antiagainst@gmail.com>
antiagainst
force-pushed
the
reland-vulkan-target
branch
from
498521f
to
efaeead
Compare
Abbreviated Benchmark Summary@ commit 8291dfe5797d4cc41a29cfc2c8ac1c701e05be7c (vs. base 7c410492bdcbfdd4972de827d995bfb8482841a5) Regressed Latencies 🚩
No improved or regressed compilation metrics 🏖️ For more information: |
Signed-off-by: Lei Zhang <antiagainst@gmail.com>
Signed-off-by: Lei Zhang <antiagainst@gmail.com>
antiagainst
force-pushed
the
reland-vulkan-target
branch
from
f96bc7d
to
13328fb
Compare
|
(typo in PR title |
ScottTodd
changed the title
This reverts commit 13328fb. Signed-off-by: Lei Zhang <antiagainst@gmail.com>
Signed-off-by: Lei Zhang <antiagainst@gmail.com>
antiagainst
force-pushed
the
reland-vulkan-target
branch
from
4037a1e
to
7c310ec
Compare
antiagainst
requested review from
kuhar,
MaheshRavishankar,
qedawkins,
Groverkss,
benvanik and
ScottTodd
as code owners
antiagainst
removed request for
benvanik,
MaheshRavishankar,
qedawkins and
Groverkss
|
@ScottTodd @kuhar this should be good now--android tests and benchmarks are fixed. |
ScottTodd
reviewed
Jun 19, 2024
This reverts commit e1d881f. Signed-off-by: Lei Zhang <antiagainst@gmail.com>
ScottTodd
reviewed
Jun 19, 2024
Signed-off-by: Lei Zhang <antiagainst@gmail.com>
kuhar
approved these changes
Jun 19, 2024
ScottTodd
approved these changes
Jun 19, 2024
|
Looks |
That runner went offline for maintenance yeah. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This relands #17623.
This commit switches SPIR-V side to use the common
#iree_gpu.targetto describe the GPU characteristics. With it we can now remove the ad-hoc Vulkan attributes and dialects and unify how GPU are described across various GPU compiler backends in IREE.SPIR-V has some additional requirements that we need to account for:
We have many vendors and APIs to handle there so this commit adds various AMD/ARM/NVIDIA/Qualcomm targets for
development purposes so that we can specify them with a shorthand.
In order to be extensible, leverage the
featurefield in#iree_gpu.targetto specify additional capabilities withcap:prefix and extensions withext:prefix. We also use thefeaturefield to specify what SPIR-V version to target with thespirv:v1.xformat.Right now the
SPIRVConvertGPUTargetpass isinvoked immediately before configuration. This is to stage the changes. As a next step we need to move
it immediately before
ConvertToSPIRVpass.--iree-vulkan-target-envis dropped given now we removed the whole Vulkan dialect and cannot control with a#vk.target_envattribute anymore.The default
--iree-vulkan-target-triplenow becomesvp_android_baseline_2022, which is a a good lowest common denominator to guarantee the generated SPIR-V is widely accepted. We are not considering SwiftShader now anymore like previously due to testing purposes.The
--iree-vulkan-target-tripleshould be renamed given it's not a triple anymore--that will happen later together with other GPU backends (i.e., cuda/hip) to be consistent.In order to support cooperative matrix conversion, we added
WMMA_F16_16x16x16_F16. For NVIDIA GPUswe are abusing it right now without considering the concrete explicit layout--that is fine given in Vulkan they are opaque anyway. But this need to be fixed if we are targeting WMMA in CUDA.
We now contruct a
#iree_gpu.targetto specifythe target to drive SPIR-V CodeGen.
Progress towards #16341
ci-extra: test_nvidia_gpu,test_nvidia_a100,test_amd_mi250, build_test_all_macos_arm64,build_and_test_android,test_on_moto-edge-x30