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.

Sign up for GitHub

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

Jump to bottom

[SPIRV] Empty struct in cbuffer causes compilation to fail #2882

Closed
baldurk opened this issue May 13, 2020 · 2 comments · Fixed by #6635
Closed

[SPIRV] Empty struct in cbuffer causes compilation to fail #2882

baldurk opened this issue May 13, 2020 · 2 comments · Fixed by #6635
Assignees
@s-perron
Labels
spirv Work related to SPIR-V

Comments

@baldurk
Copy link
Contributor

baldurk commented May 13, 2020

If an empty struct is declared in a cbuffer (or a nested struct inside) the compilation seems to break trying to align members properly to their alignment requirements around that struct:

struct empty_struct { };

cbuffer cb
{
	float3 a;
	empty_struct b;
	float2 c;

	float4 test;
};

float4 main() : SV_TARGET
{
	return test;
}

When compiling with latest dxc (dxcompiler.dll: 1.6 - 1.5.0.2653 (f0d4419f)) I get this error:

fatal error: generated SPIR-V is invalid: Structure id 4 decorated as Block for variable in Uniform storage class must follow relaxed uniform buffer layout rules: member 1 at offset 12 is not aligned to 16
  %type_cb = OpTypeStruct %v3float %empty_struct %v2float %v4float

note: please file a bug report on https://github.com/Microsoft/DirectXShaderCompiler/issues with source code if possible

With the offset of 12 coming wrongly from the float2 c trying to pack directly after float3 a. If I allow scalar layout with -fvk-use-scalar-layout then it doesn't complain since a tight packing is valid.
@ehsannas
Copy link
Contributor

Thanks for reporting @baldurk

@ehsannas ehsannas added the spirv Work related to SPIR-V label May 19, 2020
@jeremy-lunarg
Copy link
Contributor

This is an old issue, but I took a look at it. The empty struct is being assigned an offset of 12 by DXC. Relevant spir-v:

    OpMemberName %type_cb 0 "a"
    OpMemberName %type_cb 1 "b"
    OpMemberName %type_cb 2 "c"
    OpMemberName %type_cb 3 "test"
    OpMemberDecorate %type_cb 0 Offset 0
    OpMemberDecorate %type_cb 1 Offset 12
    OpMemberDecorate %type_cb 2 Offset 16
    OpMemberDecorate %type_cb 3 Offset 32

For reference glslang produces the following offsets:

    OpMemberDecorate %cb 0 Offset 0
    OpMemberDecorate %cb 1 Offset 16
    OpMemberDecorate %cb 2 Offset 16
    OpMemberDecorate %cb 3 Offset 32

That seems reasonable. Using -fvk-use-scalar-layout in DXC also produces reasonable offsets:

    OpMemberDecorate %type_cb 0 Offset 0
    OpMemberDecorate %type_cb 1 Offset 12
    OpMemberDecorate %type_cb 2 Offset 12
    OpMemberDecorate %type_cb 3 Offset 20

However, while investigating the spir-v spec, I noticed the following validation rule regarding composite objects:

"- The ArrayStride, MatrixStride, and Offset decorations must be large enough to hold the size of
the objects they affect (that is, specifying overlap is invalid). Each ArrayStride and MatrixStride
must be greater than zero, and it is invalid for two members of a given structure to be assigned the
same Offset."

Am I misinterpreting the spec?

@s-perron s-perron self-assigned this May 17, 2024
s-perron added a commit to s-perron/DirectXShaderCompiler that referenced this issue May 17, 2024
@s-perron
Fix aligment for empty structs
8112546 
We have a special case to that the the size and alignment for an empty
struct is `{1,0}`. However that is not correct. See
https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#interfaces-alignment-requirements.

> An empty structure has a base alignment equal to the size of the smallest
scalar type permitted by the capabilities declared in the SPIR-V module. (e.g.,
for a 1 byte aligned empty struct in the StorageBuffer storage class,
StorageBuffer8BitAccess or UniformAndStorageBuffer8BitAccess must be declared
in the SPIR-V module.

I'm not 100% sure how DXC handle this minimum alignment, but I figured I
would inialize the alignment to 1. If there are not members, then it
will remain 1, and I would let the rest of the logic happen. No special
case.

Fixes microsoft#2882
@s-perron s-perron mentioned this issue May 17, 2024
Merged
s-perron added a commit that referenced this issue May 22, 2024
@s-perron
Fix aligment for empty structs (#6635)
86da226 
We have a special case to that the the size and alignment for an empty
struct is `{1,0}`. However that is not correct. See

https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#interfaces-alignment-requirements.

> An empty structure has a base alignment equal to the size of the
smallest
scalar type permitted by the capabilities declared in the SPIR-V module.
(e.g.,
for a 1 byte aligned empty struct in the StorageBuffer storage class,
StorageBuffer8BitAccess or UniformAndStorageBuffer8BitAccess must be
declared
in the SPIR-V module.

I'm not 100% sure how DXC handle this minimum alignment, but I figured I
would inialize the alignment to 1. If there are not members, then it
will remain 1, and I would let the rest of the logic happen. No special
case.

Fixes #2882
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@s-perron s-perron

Labels
spirv Work related to SPIR-V
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix aligment for empty structs s-perron/DirectXShaderCompiler
4 participants
@baldurk @ehsannas @jeremy-lunarg @s-perron