[Vulkan][Runtime] Uniform buffer bugfix, minor cleanup - #7966
Merged
Conversation
Contributor
Author
|
Potential reviewers: @masahi @tmoreau89 |
tmoreau89
reviewed
May 3, 2021
Caused segfault when running on NVidia GPUs, with models that required uniform buffer arguments for constants.
Previously, these tests would show success if USE_VULKAN=OFF. Now, they correctly show that they are skipped instead.
- Explicitly require int64 support at device creation time, since the TVM-generated shaders require it. - Allocate an appropriate pool size for the buffer inputs, including both uniform and storage buffers.
tmoreau89
reviewed
May 3, 2021
tmoreau89
left a comment
tmoreau89
left a comment
Contributor
There was a problem hiding this comment.
Thank you for the bug fix @Lunderberg ! I'll wait for @masahi to do a more thorough pass, but at first glance, the changes LGTM!
masahi
reviewed
May 3, 2021
Member
|
I confirmed that this patch works on AMD too. |
…codegen_vulkan.py
Member
|
Thanks @Lunderberg @tmoreau89 |
umangyadav
pushed a commit
to umangyadav/tvm
that referenced
this pull request
May 5, 2021
* Bugfix, missing decoration on uniform buffer arguments. Caused segfault when running on NVidia GPUs, with models that required uniform buffer arguments for constants. * Updated test_target_codegen_spirv.py to use @tvm.testing.requires_vulkan Previously, these tests would show success if USE_VULKAN=OFF. Now, they correctly show that they are skipped instead. * Minor cleanup on the vulkan runtime. - Explicitly require int64 support at device creation time, since the TVM-generated shaders require it. - Allocate an appropriate pool size for the buffer inputs, including both uniform and storage buffers. * [Vulkan][Tests] Merged test_target_codegen_spirv.py into test_target_codegen_vulkan.py Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
trevor-m
pushed a commit
to trevor-m/tvm
that referenced
this pull request
May 6, 2021
* Bugfix, missing decoration on uniform buffer arguments. Caused segfault when running on NVidia GPUs, with models that required uniform buffer arguments for constants. * Updated test_target_codegen_spirv.py to use @tvm.testing.requires_vulkan Previously, these tests would show success if USE_VULKAN=OFF. Now, they correctly show that they are skipped instead. * Minor cleanup on the vulkan runtime. - Explicitly require int64 support at device creation time, since the TVM-generated shaders require it. - Allocate an appropriate pool size for the buffer inputs, including both uniform and storage buffers. * [Vulkan][Tests] Merged test_target_codegen_spirv.py into test_target_codegen_vulkan.py Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
trevor-m
pushed a commit
to trevor-m/tvm
that referenced
this pull request
May 6, 2021
* Bugfix, missing decoration on uniform buffer arguments. Caused segfault when running on NVidia GPUs, with models that required uniform buffer arguments for constants. * Updated test_target_codegen_spirv.py to use @tvm.testing.requires_vulkan Previously, these tests would show success if USE_VULKAN=OFF. Now, they correctly show that they are skipped instead. * Minor cleanup on the vulkan runtime. - Explicitly require int64 support at device creation time, since the TVM-generated shaders require it. - Allocate an appropriate pool size for the buffer inputs, including both uniform and storage buffers. * [Vulkan][Tests] Merged test_target_codegen_spirv.py into test_target_codegen_vulkan.py Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
trevor-m
pushed a commit
to trevor-m/tvm
that referenced
this pull request
May 6, 2021
* Bugfix, missing decoration on uniform buffer arguments. Caused segfault when running on NVidia GPUs, with models that required uniform buffer arguments for constants. * Updated test_target_codegen_spirv.py to use @tvm.testing.requires_vulkan Previously, these tests would show success if USE_VULKAN=OFF. Now, they correctly show that they are skipped instead. * Minor cleanup on the vulkan runtime. - Explicitly require int64 support at device creation time, since the TVM-generated shaders require it. - Allocate an appropriate pool size for the buffer inputs, including both uniform and storage buffers. * [Vulkan][Tests] Merged test_target_codegen_spirv.py into test_target_codegen_vulkan.py Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
trevor-m
pushed a commit
to neo-ai/tvm
that referenced
this pull request
May 11, 2021
* Bugfix, missing decoration on uniform buffer arguments. Caused segfault when running on NVidia GPUs, with models that required uniform buffer arguments for constants. * Updated test_target_codegen_spirv.py to use @tvm.testing.requires_vulkan Previously, these tests would show success if USE_VULKAN=OFF. Now, they correctly show that they are skipped instead. * Minor cleanup on the vulkan runtime. - Explicitly require int64 support at device creation time, since the TVM-generated shaders require it. - Allocate an appropriate pool size for the buffer inputs, including both uniform and storage buffers. * [Vulkan][Tests] Merged test_target_codegen_spirv.py into test_target_codegen_vulkan.py Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
Lunderberg
added a commit
to Lunderberg/tvm
that referenced
this pull request
May 20, 2021
spvValidate found the bug that was fixed in apache#7966, along with a few other issues on missing capability/extension declarations. Now that all unit tests pass with it enabled, would like to enable by default.
trevor-m
pushed a commit
to trevor-m/tvm
that referenced
this pull request
Jun 17, 2021
…ion (apache#8098) spvValidate found the bug that was fixed in apache#7966, along with a few other issues on missing capability/extension declarations. Now that all unit tests pass with it enabled, would like to enable by default. Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
trevor-m
pushed a commit
to neo-ai/tvm
that referenced
this pull request
Jun 17, 2021
…ion (apache#8098) spvValidate found the bug that was fixed in apache#7966, along with a few other issues on missing capability/extension declarations. Now that all unit tests pass with it enabled, would like to enable by default. Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Bugfix, missing decoration on uniform buffer arguments.
Caused segfault when running on NVidia GPUs, with models that required uniform buffer arguments for constants (>128 bytes of constants).
Updated test_target_codegen_spirv.py to use @tvm.testing.requires_vulkan
Previously, these tests would show success if USE_VULKAN=OFF. Now, they correctly show that they are skipped instead.
Explicitly require int64 support at device creation time, since the TVM-generated shaders require it.
Allocate an appropriate descriptor set pool size for the buffer inputs, including both uniform and storage buffers, when not using the push_descriptor extension.