KhronosGroup / SPIRV-Reflect

SPIRV-Reflect is a lightweight library that provides a C/C++ reflection API for SPIR-V shader bytecode in Vulkan applications.
Apache License 2.0
685 stars 148 forks source link

Out of order offsets compute incorrect padding sizes #262

Closed EllarBooher closed 5 months ago

EllarBooher commented 6 months ago

Hello, thanks a lot for this tool, it's helping me a lot with learning SPIR-V and Vulkan. With this GLSL:

#version 460
#extension GL_EXT_scalar_block_layout : require

layout(binding = 0, std430) uniform UniformBufferObject {
    layout(offset = 0) float a;
    layout(offset = 4) float b;
} ubo;

layout(binding = 0, std430) uniform UniformBufferObject2 {
    layout(offset = 4) float b;
    layout(offset = 0) float a;
} ubo2;

void main()
{
}

Compiled and reflected:

glslc -g -x glsl --target-env=vulkan1.3 -std=460 -fshader-stage=compute -O0 example.comp -o example.comp.spv
spirv-reflect example.comp.spv -y -v 1

I get the following output from spirv-reflect:

generator       : Google Shaderc over Glslang
source lang     : GLSL
source lang ver : 460
source file     : example.comp
entry point     : main (stage=CS)
local size      : (1, 1, 1)

  Descriptor bindings: 2

    Binding 0.0
      spirv id : 10
      set      : 0
      binding  : 0
      type     : VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER (CBV)
      count    : 1
      accessed : false
      name     : ubo (UniformBufferObject)
          // size = 16, padded size = 16
          struct UniformBufferObject {
              float a; // abs offset = 0, rel offset = 0, size = 4, padded size =  4 UNUSED
              float b; // abs offset = 4, rel offset = 4, size = 4, padded size = 12 UNUSED
          } ubo;

    Binding 0.0
      spirv id : 13
      set      : 0
      binding  : 0
      type     : VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER (CBV)
      count    : 1
      accessed : false
      name     : ubo2 (UniformBufferObject2)
          // size = 16, padded size = 16
          struct UniformBufferObject2 {
              float b; // abs offset = 4, rel offset = 4, size = 4, padded size = 4294967292 UNUSED
              float a; // abs offset = 0, rel offset = 0, size = 4, padded size =         16 UNUSED
          } ubo2;

I expect both to produce the same result- padded sizes of 4 for a and 12 for b since that is how the structure will be laid out in memory. I have tracked the issue to the following lines, which processes the offsets with no consideration for if the members are out of order. https://github.com/KhronosGroup/SPIRV-Reflect/blob/b50ffbda954270ddf29a355cb727f958825bcf98/spirv_reflect.c#L2714-L2725

spencer-lunarg commented 5 months ago

Thanks for raising this, can confirm this is just an issue of not checking for the struct index and relying the offset were in the same order

The simple example you gave shows this error

OpMemberDecorate %struct 0 Offset 4
OpMemberDecorate %struct 1 Offset 0

about to try and get a fix now