KhronosGroup / KhronosGroup/SPIRV-Reflect

What is the push-constant size of `OpTypePointer`?

Open
#170 5 comments 1 reaction 0 assignees View on GitHub
Dominant language
C
Stars
871
Forks
188
Avg merge
19m
Merged PRs (30d)
1

Description

Over at [`rspirv-reflect`](https://github.com/Traverse-Research/rspirv-reflect) (Rust version of `SPIRV-Reflect` built on top of [`rspirv`](https://github.com/gfx-rs/rspirv)) we [received a report](https://github.com/Traverse-Research/rspirv-reflect/issues/29) 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`](https://github.com/Traverse-Research/rspirv-reflect/pull/5) 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`:

```glsl
#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](https://github.com/KhronosGroup/SPIRV-Reflect/blob/a7c7b8a99f8fa7e21ec37f591a427196262659c4/spirv_reflect.c#L2422-L2424) (`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` 😅):

https://github.com/KhronosGroup/SPIRV-Reflect/blob/a7c7b8a99f8fa7e21ec37f591a427196262659c4/spirv_reflect.c#L2413

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`:

```glsl
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?

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in spirv_reflect.c at ParseDescriptorBlockVariableSizes() and the existing handling around SpvOpTypePointer. Reproduce the two provided GLSL push-constant cases, then determine how pointer members should contribute to reflected block size and whether the SPIR-V specification documents that size; done means the reported cases produce the expected sizes.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
computer-graphics
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.