diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2019-01-24 11:31:35 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2019-01-24 11:31:35 -0800 |
| commit | eda82cbfb92d45fbbff8dfd990798605d277608a (patch) | |
| tree | 511d10d0584fe3e580e0235d8e6aaf133e7b2513 /tests/cross-compile | |
| parent | 968d7b4084fd2e8561b7d5e65f359b05dc3d7c78 (diff) | |
Fix a regression in geometry shader cross-compilation (#794)
The underlying problem here was that legalization of entry point parameters for GLSL eliminates all the parameters to `main()`, but we still left a dangling reference to one of those parameters if it was a geometry shader output stream. The un-parented parameter would lead to an infinite loop in a later IR step, because it would never be reached by the transformation, and thus could never change its status to the one for "visited" instructions.
The fix here is to simply replace any refernces to the GS input stream parameter with an `undefined` instruction in the IR, and then rely on the fact that the downstream GLSL emit logic wouldn't actually reference that value anyway (hence why the danlging reference wasn't originally an issue).
I included a basic cross-compilation test case for geometry shaders to try to avoid subsequent regressions like this (Vulkan GS support is one of the most commonly recurring regressions we've had).
The comment I put into the IR legalization logic makes it clear that the strategy used there isn't 100% rock-solid anyway (it only works in all the `EmitVertex()` calls come from the shader entry point function, and not subroutines. Adding a better (more robust) translation strategy for geometry shaders would be a nice bit of future work.
Diffstat (limited to 'tests/cross-compile')
| -rw-r--r-- | tests/cross-compile/geometry-shader.slang | 33 | ||||
| -rw-r--r-- | tests/cross-compile/geometry-shader.slang.glsl | 96 |
2 files changed, 129 insertions, 0 deletions
diff --git a/tests/cross-compile/geometry-shader.slang b/tests/cross-compile/geometry-shader.slang new file mode 100644 index 000000000..614d71db9 --- /dev/null +++ b/tests/cross-compile/geometry-shader.slang @@ -0,0 +1,33 @@ +// geometry-shader.slang + +//TEST:CROSS_COMPILE: -profile sm_5_0 -stage geometry -entry main -target spirv-assembly + +struct CoarseVertex +{ + float4 position : POSITION; + float3 color : COLOR; + uint id : ID; +} + +struct RasterVertex +{ + float4 position : POSITION; + float3 color : COLOR; + uint id : SV_RenderTargetArrayIndex; +} + +[maxvertexcount(3)] +void main( + triangle CoarseVertex coarseVertices[3], + inout TriangleStream<RasterVertex> outputStream) +{ + for(int ii = 0; ii < 3; ++ii) + { + CoarseVertex coarseVertex = coarseVertices[ii]; + RasterVertex rasterVertex; + rasterVertex.position = coarseVertex.position; + rasterVertex.color = coarseVertex.color; + rasterVertex.id = coarseVertex.id; + outputStream.Append(rasterVertex); + } +} diff --git a/tests/cross-compile/geometry-shader.slang.glsl b/tests/cross-compile/geometry-shader.slang.glsl new file mode 100644 index 000000000..cdd0f94de --- /dev/null +++ b/tests/cross-compile/geometry-shader.slang.glsl @@ -0,0 +1,96 @@ +#version 450 +// geometry-shader.slang.glsl +//TEST_IGNORE_FILE: + +#define RasterVertex RasterVertex_0 +#define CoarseVertex CoarseVertex_0 + +#define input_position _S1 +#define input_color _S2 +#define input_id _S3 +#define output_position _S4 +#define output_color _S5 + +layout(row_major) uniform; +layout(row_major) buffer; + +layout(location = 0) +in vec4 input_position[3]; + +layout(location = 1) +in vec3 input_color[3]; + +layout(location = 2) +in uint input_id[3]; + +layout(location = 0) +out vec4 output_position; + +layout(location = 1) +out vec3 output_color; + +struct RasterVertex +{ + vec4 position_0; + vec3 color_0; + uint id_0; +}; + +struct CoarseVertex +{ + vec4 position_1; + vec3 color_1; + uint id_1; +}; + + +layout(max_vertices = 3) out; +layout(triangles) in; +layout(triangle_strip) out; + +void main() +{ + int ii_0; + + // TODO: Having to make this copy to transpose things is unfortunate. + // + // The front-end should be able to generate code using aggregate + // types for the input, and/or eliminate the redundant temporary + // by indexing directly into the sub-arrays. + // + CoarseVertex_0 _S6[3] = { + CoarseVertex_0(input_position[0], input_color[0], input_id[0]), + CoarseVertex_0(input_position[1], input_color[1], input_id[1]), + CoarseVertex_0(input_position[2], input_color[2], input_id[2]) + }; + + ii_0 = 0; + for(;;) + { + if(ii_0 < 3) + {} + else + { + break; + } + + CoarseVertex_0 coarseVertex_0 = _S6[ii_0]; + + RasterVertex_0 rasterVertex_0; + rasterVertex_0.position_0 = coarseVertex_0.position_1; + rasterVertex_0.color_0 = coarseVertex_0.color_1; + rasterVertex_0.id_0 = coarseVertex_0.id_1; + + RasterVertex_0 _S7 = rasterVertex_0; + + output_position = _S7.position_0; + output_color = _S7.color_0; + gl_Layer = int(_S7.id_0); + + EmitVertex(); + + ii_0 = ii_0 + 1; + } + + return; +} |
