What is the push-constant size of OpTypePointer?

[MarijnS95]

Over at rspirv-reflect (Rust version of SPIRV-Reflect built on top of rspirv) we received a report about not computing the right/expected push constant size for GLSL buffer references (our version of spvReflectEnumeratePushConstantBlocks() and ParseDescriptorBlockVariableSizes()). For the time being I hardcoded a size of 8 but now decided to confirm with SPIRV-Reflect before committing to that, given that the SPIR-V spec doesn't mention anything in this area.

And as it turns out SPIRV-Reflect doesn't handle this case either. It seemed to work at first (surprisingly given there's no SpvOpTypePointer handling inside ParseDescriptorBlockVariableSizes()) with the provided test-case returning 16 which is expected if the pointer size is 8:

#extension GL_EXT_buffer_reference:require
#extension GL_EXT_buffer_reference2:require

struct Mesh {
    vec4 position;
    vec2 uv;
};

layout(std430, buffer_reference, buffer_reference_align = 32) readonly buffer MeshBuffer {
    Mesh mesh[];
};
layout(std430, buffer_reference) readonly buffer IndexBuffer {
    uint index[];
};

layout(push_constant) uniform registers {
    MeshBuffer mesh_buffer;
    IndexBuffer index_buffer;
} Registers;

But soon found out that SPIRV-Reflect is using the same trick to grab the offset of the last member (8 here) so that it only has to compute the size of the last member... and rounds that up to SPIRV_DATA_SIZE=16 (on a related note: this alignment is something we should handle in rspirv-reflect 😅):

SPIRV-Reflect/spirv_reflect.c

Line 2413 in a7c7b8a

p_member_var->padded_size = RoundUp(p_member_var->offset + p_member_var->size, SPIRV_DATA_ALIGNMENT) - p_member_var->offset;

This of course falls flat on its face when the offset becomes equal to the rounding (and p_member_var->size remains zero), also returning a push constant size of 16 for the following even though it should be 24:

layout(push_constant) uniform Registers
{
    uint test; // off=0, size=4
    MeshBuffer mesh_buffer; // off=8, size=8?
    IndexBuffer index_buffer; // off=16, size=8?
}
registers;

---

Effectively this unnecessarily-verbose backstory boils down to two questions:

1. What is the size of OpTypePointer (the shader compiler knows because it can compute the offsets)?
   • And is there documentation specifying this we can link to?
2. Can we add an extra case SpvOpTypePointer: to SPIRV-Reflect to support this use-case?

[chaoticbob]

Hi - thanks for for the complete backstory and repro case :)

Answering your questions:

1. I'll need to consult some folks on the sizing of OpTypePointer. I was always under the impression that its final resolved type would be the size but maybe this is incorrrect.
2. Possibly. Will investigate.

[chaoticbob]

Quick update thanks to (@s-perron):
https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_physical_storage_buffer.asciidoc

This case is not currently handled by spirv-reflect, so to answer the question you posed in #2. Yes, it can and will need to be added.

[MarijnS95]

@chaoticbob thanks! So according to https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_physical_storage_buffer.asciidoc#overview:

This extension adds a new storage class PhysicalStorageBuffer which is similar to StorageBuffer except pointers to the PhysicalStorageBuffer storage class are treated as physical pointer types according to a new addressing model PhysicalStorageBuffer64. This addressing model is a hybrid of logical and physical addressing, with only pointers to PhysicalStorageBuffer storage class being physical, and using 64-bit addresses.

There's a single addressing model provided by this extension and it always uses 64-bit pointers. However, the docs don't mention OpTypePointer specifically anywhere.

Then, the user asked this question in the context of GL_EXT_buffer_reference (which results in OpCapability PhysicalStorageBufferAddresses, OpExtension "SPV_KHR_physical_storage_buffer", OpMemoryModel PhysicalStorageBuffer64 GLSL450 anyway) whose docs state:

A corresponding API extension provides functionality to retrieve the address value for a given buffer, and also defines the size and alignment of the reference types.

So the size will be defined by an API extension. Though that "API extension" probably isn't very relevant since it is said to map to SPV_EXT_physical_storage_buffer (or SPV_KHR_physical_storage_buffer?) extension, which itself defines the type as linked above:

Add to the "Mapping to SPIR-V" section

Uses the SPV_EXT_physical_storage_buffer extension to define reference types as pointer types to the PhysicalStorageBufferEXT storage class.

---

In short, for OpTypePointer PhysicalStorageBuffer T, when the memory model is PhysicalStorageBuffer64, the size is 8. Other combinations are unsupported (for now).

[s-perron]

So the size will be defined by an API extension.

The API extension is https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_buffer_device_address.html. It says 64-bit addresses, but does not mention the alignment, but it references https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkDeviceAddress.html, which says the device address is a uint64_t.

In short, for OpTypePointer PhysicalStorageBuffer T, when the memory model is PhysicalStorageBuffer64, the size is 8. Other combinations are unsupported (for now).

I would rephrase as "Other combinations are not allowed to be in interface variable in Vulkan shader, so they are irrelevant to sprv-reflect."

[MarijnS95]

The API extension is https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_buffer_device_address.html. It says 64-bit addresses, but does not mention the alignment, but it references https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkDeviceAddress.html, which says the device address is a uint64_t.

Right, VK_KHR_buffer_device_address interacts very closely with (requires) SPV_KHR_physical_storage_buffer, and both define the same.

Compiling:

layout(push_constant) uniform Registers
{
    uint foo;
    MeshBuffer mesh_buffer;
    IndexBuffer index_buffer;
}
registers;

results in:

OpMemberDecorate %Registers 0 Offset 0
OpMemberDecorate %Registers 1 Offset 8
OpMemberDecorate %Registers 2 Offset 16

So I assume the alignment is 8 too.

I would rephrase as "Other combinations are not allowed to be in interface variable in Vulkan shader, so they are irrelevant to sprv-reflect."

Ensuring the OpTypePointer is only used as part of an interface variable is another validation step we could perform, but in the current context (r)spirv-reflect only computes/provides struct sizes for push constants anyway.