Fix a regression in geometry shader cross-compilation (shader-slang#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.

Author: Tim Foley

source/slang/ir-glsl-legalize.cpp

@@ -1238,6 +1238,23 @@ void legalizeEntryPointParameterForGLSL(
                 }
             }
 
+            // We will still have references to the parameter coming
+            // from the `EmitVertex` calls, so we need to replace it
+            // with something. There isn't anything reasonable to
+            // replace it with that would have the right type, so
+            // we will replace it with an undefined value, knowing
+            // that the emitted code will not actually reference it.
+            //
+            // TODO: This approach to generating geometry shader code
+            // is not ideal, and we should strive to find a better
+            // approach that involes coding the `EmitVertex` operation
+            // directly in the stdlib, similar to how ray-tracing
+            // operations like `TraceRay` are handled.
+            //
+            builder->setInsertBefore(func->getFirstBlock()->getFirstOrdinaryInst());
+            auto undefinedVal = builder->emitUndefined(pp->getFullType());
+            pp->replaceUsesWith(undefinedVal);
+
             return;
         }
     }

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);
+    }
+}

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;
+}