Repository navigation
Support SSBO POD kernel arguments with physical storage buffers - #1690
Merged
Merged
Conversation
dneto0
reviewed
Oct 22, 2026
| return ArgKind::PointerPushConstant; | ||
| case kSSBO: | ||
| llvm_unreachable("SSBO PODs not supported with physical pointer arguments!"); | ||
| return ArgKind::PointerUBO; |
Collaborator
There was a problem hiding this comment.
This reads weirdly. I see that it's the only pointer-* kind that is not a pointer_pushconstant. But it seems strange to name it as a UBO when it's an SSBO ?
I'd like @alan-baker 's take on this.
I'm sure this is functional.
Collaborator
There was a problem hiding this comment.
I agree it forces a double take when you read it. Might be beneficial to merge this with kUBO and have an explanatory comment.
Collaborator
Author
There was a problem hiding this comment.
I've added a comment to make it clearer
When -physical-storage-buffers is enabled and POD arguments fall back to kSSBO (for example due to 8-bit/16-bit UBO and push constant storage restrictions), clspv previously hit an unreachable in GetArgKindForPointerPodArgs and mapped ArgKind::Pod descriptor variables to AddressSpace::Global, which SPIRVProducerPass emits as StorageClassPhysicalStorageBuffer instead of StorageClassStorageBuffer. Add AddressSpace::StorageBuffer for explicit SSBO descriptor variables so ArgKind::Pod variables use StorageClassStorageBuffer even when physical storage buffers are enabled, and return ArgKind::PointerUBO for kSSBO in GetArgKindForPointerPodArgs. Add a PhysicalStorageBuffers lit test covering clustered, untyped-pointer, and non-clustered SSBO POD arguments. Change-Id: I7a343939fa5a8cb46bf78a03d660618ecb4c52f3
dneto0
approved these changes
Oct 23, 2026
dneto0
left a comment
dneto0
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the update.
The comment explains the challenge to the reader's expectation, and is almost a TODO for reconsideration later on.
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
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.
When -physical-storage-buffers is enabled and POD arguments fall back to kSSBO (for example due to 8-bit/16-bit UBO and push constant storage restrictions), clspv previously hit an unreachable in GetArgKindForPointerPodArgs and mapped ArgKind::Pod descriptor variables to AddressSpace::Global, which SPIRVProducerPass emits as StorageClassPhysicalStorageBuffer instead of StorageClassStorageBuffer.
Add AddressSpace::StorageBuffer for explicit SSBO descriptor variables so ArgKind::Pod variables use StorageClassStorageBuffer even when physical storage buffers are enabled, and return ArgKind::PointerUBO for kSSBO in GetArgKindForPointerPodArgs. Add a PhysicalStorageBuffers lit test covering clustered, untyped-pointer, and non-clustered SSBO POD arguments.