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

[SPIR-V] Arrays of empty structs in cbuffers produce validation errors when optimizations are disabled (asks to file a bug) #3101

Closed
Dredhog opened this issue Aug 27, 2020 · 1 comment · Fixed by #3885
Labels
spirv Work related to SPIR-V

Comments

@Dredhog
Copy link

Dredhog commented Aug 27, 2020

Title

Having an array of empty structs in a cbuffer produces a validation error when compiling without optimizations. Error message asks to report issue.

Functional impact

Compilation fails only with optimization level 0 i.e. when using compilation flag -O0 or -Od

Minimal repro steps

  1. Compile the following pixel shader with dxc.exe -T ps_6_0 -E Frag repro_shader.hlsl -spirv -O0
struct PSInput
{
	float4 color : COLOR;
};

cbuffer UnityInstancing_PerDraw1 {
    struct {
	} unity_Builtins1Array[2];
}

float4 PSMain(PSInput input) : SV_TARGET
{
	return input.color;
}

Observe printed error message:

fatal error: generated SPIR-V is invalid: Structure id 165 decorated as Block for variable in Uniform storage class must follow relaxed uniform buffer layout rules: member 0 contains an array with stride 0
  %type_UnityInstancing_PerDraw1 = OpTypeStruct %_arr__struct_167_uint_2

note: please file a bug report on https://github.com/Microsoft/DirectXShaderCompiler/issues with source code if possible
  1. Note: the issue was discovered while using scalar block layout which is more relevant to us at Unity:
    the error produced is appropriately different:
fatal error: generated SPIR-V is invalid: Structure id 165 decorated as Block for variable in Uniform storage class must follow scalar uniform buffer layout rules: member 0 contains an array with stride 0
  %type_UnityInstancing_PerDraw1 = OpTypeStruct %_arr__struct_167_uint_2

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

Here's a quick reproduction on shader-playground

Expected result

Compilation succeeds and works since the DXIL backend works with -O0. In DXIL the empty cbuffer seems to be removed if unused (which might not be an option) but even if the empty struct array is put into a used cbuffer it is actually correctly identified (marked with /* empty struct */ see example)

Actual result

Compilation fails with error seen in the reproduction steps. Error message asks to report the issue.

Further technical details

Reproduces also on most recent source builds of DXC 530958e

@Dredhog Dredhog changed the title [SPIR-V] Array of empty structs produces validation error when optimizations are disabled (asks to file a bug) [SPIR-V] Arrays of empty structs in cbuffers produce validation errors when optimizations are disabled (asks to file a bug) Aug 27, 2020
@ehsannas ehsannas added the spirv Work related to SPIR-V label Aug 27, 2020
Dredhog added a commit to Unity-Technologies/Graphics that referenced this issue Sep 7, 2020
*If no comment in shader, tex+sampler emulation issue
 microsoft/DirectXShaderCompiler#3100
*A few shaders fail due to use of arrays of empty structs in cbuffers
 microsoft/DirectXShaderCompiler#3101
*AmbientProbeConvolution.compute fails to create an Metal pipeline,
 not yet sure what the issue is but produces black artifacts on lit
 objects due to bad env lighting data
@jaebaek
Copy link
Collaborator

jaebaek commented Jul 26, 2021

I am not sure we have a clear solution for this issue because it happens because of the limitation of SPIR-V.
To avoid the validation error message, we have to emit the following SPIR-V:

               OpMemberDecorate %type_UnityInstancing_PerDraw1 0 Offset 0
               OpDecorate %type_UnityInstancing_PerDraw1 Block
               OpDecorate %_arr__struct_13_uint_2 ArrayStride 16
...
%type_UnityInstancing_PerDraw1 = OpTypeStruct %_arr__struct_13_uint_2

If we do not set the ArrayStride of %_arr__struct_13_uint_2 as 16, spirv-val reports the same error, because it is a part of an object with Block decoration.

However, the ArrayStride of %_arr__struct_13_uint_2 is not 16. Since unity_Builtins1Array[2] is an empty array, its array stride must be 0. It is more clear with the following code:

struct PSInput
{
	float4 color : COLOR;
};

cbuffer UnityInstancing_PerDraw1 {
    struct {
	} unity_Builtins1Array[2];
  float4 additionalData;
}

float4 PSMain(PSInput input) : SV_TARGET
{
	return input.color;
}

If we set the ArrayStride of %_arr__struct_13_uint_2 as 16, the offset of additionalData must be 32 not 0, which is not true because unity_Builtins1Array[2] is empty.

The DXIL result sets the array size and stride as 0. We also do the same thing as 0 for the SPIR-V result and it results in the spirv-val error.

In my opinion, the best thing we can do is just disabling the spirv-val check when it has a cbuffer with an empty array and the legalization is disabled.

bob80905 pushed a commit to bob80905/DirectXShaderCompiler that referenced this issue Jun 28, 2023
Fixes microsoft#3101

error out if 2 blobs compiled with different compiler versions are linked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spirv Work related to SPIR-V
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants