Support different buffer layouts
bpeel opened this issue · 4 comments
When uploading data to a UBO, SSBO or push constant, or when probing for results, VkRunner needs to know the layout of the buffer in order to calculate the offsets of the elements. The main things it needs to know are the array stride, matrix stride and the major axis of matrices. Currently VkRunner just assumes that all buffers are in std140 layout. This only matters for arrays and matrices because the layout of primitive types is only affected by their offset and that is explicitly specified in every corresponding test command anyway. Vulkan/SPIR-V supports arbitrary strides and offsets as long as they don’t conflict with certain minimums. Different strides could be specified for each member of a struct. Under GLSL as far as I know the strides are limited to a choice between std140 and std430.
There are a few ways to implement this. We could either limit the choice to std140 or std430 or we could allow a complete specification. We could make the commands per-buffer, or global or have them specific to each command that needs them.
I think my current preference would be to limit the choice to std140 and std430 and just have a global command that sets it for all subsequent commands. We could also have a command to pick the matrix axis. So something like this:
# Use std140 to upload these elements
layout std140
col major
ssbo 3 subdata mat3 0 1 2 3 4 5 6 7 8 9
ssbo 3 subdata float 48 10 11 12
# Use std430 for these probe commands
layout std430
row major
probe ssbo mat2 4 0 ~= 1 2 3 4
probe ssbo float 4 16 ~= 5 6 7
Another option might be to have a different default option for UBOs and SSBOs because that is what GLSL has. However this might be a little bit more confusing.
Based on Vulkan spec, Vulkan allows only std140 and std430 of GLSL as its layouts.
IMHO, limiting the choice to std140 and std430 is fine.
The example commands i.e., layout std140 and layout std430 look good to me, but I am not sure we need col major and row major.
Jumping in the discussion, as I have been involved a lot on layouts due the ARB_gl_spirv work
Based on Vulkan spec, Vulkan allows only std140 and std430 of GLSL as its layouts.
IMHO, limiting the choice to std140 and std430 is fine.
What that vulkan spec section points is that std140 and std430 in GLSL satisfies Vulkan memory layout rules. But Vulkan allows to extend that. For example, on that Vulkan spec quote you can read:
An array has a base alignment equal to the base alignment of its element type, rounded up to a multiple of 16.
While on GLSL you have:
The array may have padding at the end; the base offset of the member following the array is rounded up to the next multiple of the base alignment
Note that on GLSL is the next multiple, while on Vulkan it can be a multiple of 16.
So, it is true that if your SPIR-V shaders came from a GLSL to SPIR-V translation, you would only found std140 or std430, and that's ok for Vulkan. If that is the case, then supporting only those two would be enough. If you want to test any memory layout that Vulkan allows, then it would be needed to provide a way to specify custom array/matrix strides.
More info on this spec issue:
https://gitlab.khronos.org/opengl/API/issues/92
@infapi00 Thank you for detailed explanation.
Supporting all memory layouts would be nice but we do not have specific use case.
IMO, it would be more efficient that we allow only std140 and std430 now and support others later.
I agree, we can add support for others later.