WGSL: Nested structures are not allowed for entry point IO

[aleino-nv]

This issue was split from #5173.

Affected tests under tests/compute (currently excluded):

1. compile-time-loop

Output from slang-test, for compile-time-loop:

...
WGPU error: Error while parsing WGSL: :38:6 error: nested structures cannot be used for entry point IO
     assembledVertex_0 : AssembledVertex_0,
     ^^^^^^^^^^^^^^^^^

:42:1 note: while analyzing entry point 'vertexMain'
fn vertexMain( input_0 : VertexStageInput_0) -> VertexStageOutput_0
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
{
^
    var output_0 : VertexStageOutput_0;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    output_0.coarseVertex_0.color_0 = input_0.assembledVertex_0.color_1;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
var _S2 : vec4<f32> = (((vec4<f32>(input_0.assembledVertex_0.position_0, 1.0f)) * (unpackStorage_0(Uniforms_0.modelViewProjection_0))));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    output_0.sv_position_0 = _S2;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    output_0.coarseVertex_0.uv_0 = input_0.assembledVertex_0.uv_1;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    return output_0;
^^^^^^^^^^^^^^^^^^^^
}
^


 - While validating [ShaderModuleDescriptor]
 - While calling [Device].CreateShaderModule([ShaderModuleDescriptor]).
...

Generated WGSL for slangc -o test.wgsl -target wgsl -stage vertex -entry vertexMain %SLANG_SRC%\tests\compute\compile-time-loop.slang:

...
struct AssembledVertex_0
{
     position_0 : vec3<f32>,
     color_1 : vec3<f32>,
     uv_1 : vec2<f32>,
};

struct VertexStageInput_0
{
     assembledVertex_0 : AssembledVertex_0,
};

@vertex
fn vertexMain( input_0 : VertexStageInput_0) -> VertexStageOutput_0
{
    var output_0 : VertexStageOutput_0;
    output_0.coarseVertex_0.color_0 = input_0.assembledVertex_0.color_1;
    output_0.sv_position_0 = (((vec4<f32>(input_0.assembledVertex_0.position_0, 1.0f)) * (unpackStorage_0(Uniforms_0.modelViewProjection_0))));
    output_0.coarseVertex_0.uv_0 = input_0.assembledVertex_0.uv_1;
    return output_0;
}
...

[aleino-nv]

@csyonghe The GLSL output seems to get rid of this kind of nesting. Does a similar restriction apply to GLSL?

[aleino-nv]

I think I found he passage that forbids nesting of input structures
in the section on entry point declarations (https://www.w3.org/TR/WGSL/#entry-point-decl):

"The type of each formal parameter, and the entry point’s return type, must be one of: ... a structure whose member types are any of bool, numeric scalar, or numeric vector. ..."

[aleino-nv]

Maybe we can re-use the same legalization pass that's used for GLSL to fix this WGSL issue.

[aleino-nv]

Maybe we can re-use the same legalization pass that's used for GLSL to fix this WGSL issue.

@csyonghe If this exists as a common pass somewhere, could you point me at the code for that?

[csyonghe]

I think we either need to run legalizeEntryPointForGLSL as is, or figure out a way to run the createGLSLGlobalVaryings pass defined in slang-ir-glsl-legalize.cpp. You can take a look at its implementation to see how this is done in GLSL and whether the logic there is directly applicable to wgsl.

The issue I am seeing is that there may be things in legalizeEntryPointForGLSL that is not necessary for wgsl. So the TLDR is there may not be a directly reusable pass, but what is in slang-ir-glsl-legalize.cpp should be something close to what we want.

[jkwak-work]

I think there is one more problem for this test.
When I manually removed the nested struct, it was still giving me errors saying that the variables are not bound.
The following code worked at the end,

struct VertexStageOutput_0
{
     @location(0) color_0 : vec3<f32>,
     @location(1) uv_0 : vec2<f32>,
     @builtin(position) sv_position_0 : vec4<f32>,
};

struct AssembledVertex_0
{
     @location(0) position_0 : vec3<f32>,
     @location(1) color_1 : vec3<f32>,
     @location(2) uv_1 : vec2<f32>,
};

I think @builtin(position) is never needed for compute shaders but it is needed for vertex shader, I guess.

[jkwak-work]

I think we either need to run legalizeEntryPointForGLSL as is, or figure out a way to run the createGLSLGlobalVaryings pass defined in slang-ir-glsl-legalize.cpp. You can take a look at its implementation to see how this is done in GLSL and whether the logic there is directly applicable to wgsl.

The issue I am seeing is that there may be things in legalizeEntryPointForGLSL that is not necessary for wgsl. So the TLDR is there may not be a directly reusable pass, but what is in slang-ir-glsl-legalize.cpp should be something close to what we want.

After debugging a bit, I found that this comment is exactly what the issue is.
We cannot directly call those functions because they are specific to GLSL.
I will need to copy-paste and modify it to work for WGSL.

[jkwak-work]

I like to share a bit of update.
For the return type legalization, I found that there is a little difference between GLSL and WGSL.
In GLSL, the entry point for a vertex shader returns void type and we create global out parameters.
In WGSL, the entry point for a vertex shader must return all the varying outputs; in other words, it cannot have global out parameters.

[csyonghe]

In this case, the logic we want is probably closer to the legalization for metal than for glsl.

[jkwak-work]

Although the failing test is under test/compute directory, compile-time-loop doesn't appear to be a compute test.
It requires vertex/fragment shaders to work.

There are two reasons why the test is failing, but both of them comes down to the fact that we are not properly handling "varying input" yet.

1. the binding keywords are not properly emitted for the varying input and the varying output.
2. the nested struct is not in-lined/flattened for the varying input and output.

I created a separate issue for the first problem,

• [WGSL] Binding keywords are missing for a vertex shader entry-point #5486

For the second problem, WGSL prevents any varying inputs from being nested.
The spec doesn't explicitly mentioned it but there is an example for this,

struct B {
  @location(0) x: f32
}

struct C {
  // Invalid, structures with user-defined IO cannot be nested.
  b: B
}

The problem is limited to the case where a varying input is included as a nesting.
The nesting is not a problem if the varying input/output is not involved.

Since compute shader cannot take "varying input", I don't think this test is for compute shader.
I think we should push this issue to when the vertex shader is properly working.