Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Structure layout reuse does not enumerate structure member count #230

Closed
muraj opened this issue Oct 31, 2023 · 2 comments · Fixed by #232
Closed

Structure layout reuse does not enumerate structure member count #230

muraj opened this issue Oct 31, 2023 · 2 comments · Fixed by #232
Assignees

Comments

@muraj
Copy link

muraj commented Oct 31, 2023

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 spencer-lunarg self-assigned this Nov 1, 2023
@spencer-lunarg
Copy link
Contributor

@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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants