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

Structure layout reuse does not enumerate structure member count #230

Closed muraj closed 10 months ago

muraj commented 11 months ago

The following GLSL fails spvReflectCreateShaderModule when calling ParseDescriptorBlockVariableUsage on the second member "b":

#version 460 core
#extension GL_EXT_buffer_reference : require
#extension GL_EXT_shader_explicit_arithmetic_types_float32 : require
#extension GL_EXT_shader_explicit_arithmetic_types_int64 : require
#extension GL_EXT_shader_explicit_arithmetic_types_int32 : require

layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;

layout(buffer_reference, buffer_reference_align = 4) readonly buffer ReadVecf
{
    float32_t values[];
};

layout(buffer_reference, buffer_reference_align = 4) writeonly buffer WriteVecf
{
    float32_t values[];
};

layout(push_constant, std430) uniform Parameters
{
    ReadVecf a;
    ReadVecf b;
    WriteVecf c;
    uint64_t n;
} params;

void main()
{
    uint32_t idx = gl_GlobalInvocationID.x;
    uint32_t stride = gl_NumWorkGroups.x * gl_WorkGroupSize.x;
    for (; idx < params.n; idx += stride) {
        params.c.values[idx] = params.a.values[idx] + params.b.values[idx];
    }
}

The following GLSL succeeds:

#version 460 core
#extension GL_EXT_buffer_reference : require
#extension GL_EXT_shader_explicit_arithmetic_types_float32 : require
#extension GL_EXT_shader_explicit_arithmetic_types_int64 : require
#extension GL_EXT_shader_explicit_arithmetic_types_int32 : require

layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;

layout(buffer_reference, buffer_reference_align = 4) readonly buffer ReadVecf
{
    float32_t values[];
};

layout(buffer_reference, buffer_reference_align = 4) readonly buffer ReadVecf2
{
    float32_t values[];
};

layout(buffer_reference, buffer_reference_align = 4) writeonly buffer WriteVecf
{
    float32_t values[];
};

layout(push_constant, std430) uniform Parameters
{
    ReadVecf a;
    ReadVecf2 b;
    WriteVecf c;
    uint64_t n;
} params;

void main()
{
    uint32_t idx = gl_GlobalInvocationID.x;
    uint32_t stride = gl_NumWorkGroups.x * gl_WorkGroupSize.x;
    for (; idx < params.n; idx += stride) {
        params.c.values[idx] = params.a.values[idx] + params.b.values[idx];
    }
}

I haven't narrowed down exactly why this is the case yet, but I at least have a work around for now.

spencer-lunarg commented 11 months ago

@muraj thanks for reporting! I can reproduce this crash, there has been some edge cases around GL_EXT_buffer_reference I see are not fixed. I can take a look later this week/weekend

spencer-lunarg commented 11 months ago

so took a stab quick, the core issue is we added protection from going

layout(buffer_reference) buffer t1;
layout(buffer_reference) buffer t2;

layout(buffer_reference, std430) buffer t1 {
    t2 i_1;
};

layout(buffer_reference, std430) buffer t2 {
    t1 i_2;
};

and getting into a recursion for loop

Need to make that check smarter to realize that these are 2 are called from same struct, so if one is safe, the other is as well