[naga/spv-in] Remove unnecessary "gl_PerVertex" name check so unused builtins will always be skipped

CHANGELOG.md

@@ -121,6 +121,7 @@ Bottom level categories:
 
 #### Naga
 - Make use of `GL_EXT_texture_shadow_lod` to support sampling a cube depth texture with an explicit LOD. By @cmrschwarz in #[5171](https://github.com/gfx-rs/wgpu/pull/5171).
+- In spv-in, remove unnecessary "gl_PerVertex" name check so unused builtins will always be skipped. By @Imberflur in [#5227](https://github.com/gfx-rs/wgpu/pull/5227).
 
 #### Tests
 

naga/src/front/spv/mod.rs

@@ -577,6 +577,9 @@ pub struct Frontend<I> {
     lookup_function_type: FastHashMap<spirv::Word, LookupFunctionType>,
     lookup_function: FastHashMap<spirv::Word, LookupFunction>,
     lookup_entry_point: FastHashMap<spirv::Word, EntryPoint>,
+    // When parsing functions, each entry point function gets an entry here so that additional
+    // processing for them can be performed after all function parsing.
+    deferred_entry_points: Vec<(EntryPoint, spirv::Word)>,
     //Note: each `OpFunctionCall` gets a single entry here, indexed by the
     // dummy `Handle<crate::Function>` of the call site.
     deferred_function_calls: Vec<spirv::Word>,
@@ -628,6 +631,7 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
             lookup_function_type: FastHashMap::default(),
             lookup_function: FastHashMap::default(),
             lookup_entry_point: FastHashMap::default(),
+            deferred_entry_points: Vec::default(),
             deferred_function_calls: Vec::default(),
             dummy_functions: Arena::new(),
             function_call_graph: GraphMap::new(),
@@ -1561,12 +1565,10 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
                                     span,
                                 );
 
-                                if ty.name.as_deref() == Some("gl_PerVertex") {
-                                    if let Some(crate::Binding::BuiltIn(built_in)) =
-                                        members[index as usize].binding
-                                    {
-                                        self.gl_per_vertex_builtin_access.insert(built_in);
-                                    }
+                                if let Some(crate::Binding::BuiltIn(built_in)) =
+                                    members[index as usize].binding
+                                {
+                                    self.gl_per_vertex_builtin_access.insert(built_in);
                                 }
 
                                 AccessExpression {
@@ -3956,6 +3958,12 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
             }?;
         }
 
+        // Do entry point specific processing after all functions are parsed so that we can
+        // cull unused problematic builtins of gl_PerVertex.
+        for (ep, fun_id) in core::mem::take(&mut self.deferred_entry_points) {
+            self.process_entry_point(&mut module, ep, fun_id)?;
+        }
+
         log::info!("Patching...");
         {
             let mut nodes = petgraph::algo::toposort(&self.function_call_graph, None)
@@ -5081,7 +5089,7 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
             None
         };
         let span = self.span_from_with_op(start);
-        let mut dec = self.future_decor.remove(&id).unwrap_or_default();
+        let dec = self.future_decor.remove(&id).unwrap_or_default();
 
         let original_ty = self.lookup_type.lookup(type_id)?.handle;
         let mut ty = original_ty;
@@ -5127,17 +5135,6 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
             None => map_storage_class(storage_class)?,
         };
 
-        // Fix empty name for gl_PerVertex struct generated by glslang
-        if let crate::TypeInner::Pointer { .. } = module.types[original_ty].inner {
-            if ext_class == ExtendedClass::Input || ext_class == ExtendedClass::Output {
-                if let Some(ref dec_name) = dec.name {
-                    if dec_name.is_empty() {
-                        dec.name = Some("perVertexStruct".to_string())
-                    }
-                }
-            }
-        }
-
         let (inner, var) = match ext_class {
             ExtendedClass::Global(mut space) => {
                 if let crate::AddressSpace::Storage { ref mut access } = space {

naga/tests/in/spv/builtin-accessed-outside-entrypoint.spvasm

@@ -0,0 +1,90 @@
+;; Ensure builtin binding isn't removed by unused gl_PerVertex builtin culling when
+;; the builtin is used in a function defined after (in the SPIRV) the entry point.
+;;
+;; Generated from the following glsl via `glslc` (without `-O` flag):
+;;
+;; ```glsl
+;; #version 450
+;;
+;; void builtin_usage() {
+;;     gl_Position = vec4(
+;;         (gl_VertexIndex == 0) ? -4.0 : 1.0,
+;;         (gl_VertexIndex == 2) ? 4.0 : -1.0,
+;;         0.0,
+;;         1.0
+;;     );
+;; }
+;;
+;; void main()
+;; {
+;;     builtin_usage();
+;; }
+;; ```
+;;
+; SPIR-V
+; Version: 1.0
+; Generator: Google Shaderc over Glslang; 11
+; Bound: 37
+; Schema: 0
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Vertex %main "main" %_ %gl_VertexIndex
+               OpSource GLSL 450
+               OpSourceExtension "GL_GOOGLE_cpp_style_line_directive"
+               OpSourceExtension "GL_GOOGLE_include_directive"
+               OpName %main "main"
+               OpName %builtin_usage_ "builtin_usage("
+               OpName %gl_PerVertex "gl_PerVertex"
+               OpMemberName %gl_PerVertex 0 "gl_Position"
+               OpMemberName %gl_PerVertex 1 "gl_PointSize"
+               OpMemberName %gl_PerVertex 2 "gl_ClipDistance"
+               OpMemberName %gl_PerVertex 3 "gl_CullDistance"
+               OpName %_ ""
+               OpName %gl_VertexIndex "gl_VertexIndex"
+               OpMemberDecorate %gl_PerVertex 0 BuiltIn Position
+               OpMemberDecorate %gl_PerVertex 1 BuiltIn PointSize
+               OpMemberDecorate %gl_PerVertex 2 BuiltIn ClipDistance
+               OpMemberDecorate %gl_PerVertex 3 BuiltIn CullDistance
+               OpDecorate %gl_PerVertex Block
+               OpDecorate %gl_VertexIndex BuiltIn VertexIndex
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+    %v4float = OpTypeVector %float 4
+       %uint = OpTypeInt 32 0
+     %uint_1 = OpConstant %uint 1
+%_arr_float_uint_1 = OpTypeArray %float %uint_1
+%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1 %_arr_float_uint_1
+%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
+          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+%_ptr_Input_int = OpTypePointer Input %int
+%gl_VertexIndex = OpVariable %_ptr_Input_int Input
+       %bool = OpTypeBool
+   %float_n4 = OpConstant %float -4
+    %float_1 = OpConstant %float 1
+      %int_2 = OpConstant %int 2
+    %float_4 = OpConstant %float 4
+   %float_n1 = OpConstant %float -1
+    %float_0 = OpConstant %float 0
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+         %36 = OpFunctionCall %void %builtin_usage_
+               OpReturn
+               OpFunctionEnd
+%builtin_usage_ = OpFunction %void None %3
+          %7 = OpLabel
+         %20 = OpLoad %int %gl_VertexIndex
+         %22 = OpIEqual %bool %20 %int_0
+         %25 = OpSelect %float %22 %float_n4 %float_1
+         %26 = OpLoad %int %gl_VertexIndex
+         %28 = OpIEqual %bool %26 %int_2
+         %31 = OpSelect %float %28 %float_4 %float_n1
+         %33 = OpCompositeConstruct %v4float %25 %31 %float_0 %float_1
+         %35 = OpAccessChain %_ptr_Output_v4float %_ %int_0
+               OpStore %35 %33
+               OpReturn
+               OpFunctionEnd

naga/tests/in/spv/unnamed-gl-per-vertex.spvasm

@@ -0,0 +1,73 @@
+;; Make sure that we don't have a validation error due to lacking capabilities
+;; for bulltins such as ClipDistance when those builtin are not actually used.
+;;
+;; This specifically doesn't name the gl_PerVertex struct to make sure we don't
+;; rely on checks for this name.
+;;
+;; See https://github.com/gfx-rs/wgpu/issues/4915
+;;
+;; Generated via `glslc -O` on this glsl code (taken from https://github.com/gfx-rs/wgpu/issues/4915):
+;;
+;; ```glsl
+;; #version 450
+;;
+;; void main()
+;; {
+;;     gl_Position = vec4(
+;;         (gl_VertexIndex == 0) ? -4.0 : 1.0,
+;;         (gl_VertexIndex == 2) ? 4.0 : -1.0,
+;;         0.0,
+;;         1.0
+;;     );
+;; } 
+;; ```
+;;
+; SPIR-V
+; Version: 1.0
+; Generator: Google Shaderc over Glslang; 11
+; Bound: 34
+; Schema: 0
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Vertex %4 "main" %13 %gl_VertexIndex
+               OpMemberDecorate %_struct_11 0 BuiltIn Position
+               OpMemberDecorate %_struct_11 1 BuiltIn PointSize
+               OpMemberDecorate %_struct_11 2 BuiltIn ClipDistance
+               OpMemberDecorate %_struct_11 3 BuiltIn CullDistance
+               OpDecorate %_struct_11 Block
+               OpDecorate %gl_VertexIndex BuiltIn VertexIndex
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+    %v4float = OpTypeVector %float 4
+       %uint = OpTypeInt 32 0
+     %uint_1 = OpConstant %uint 1
+%_arr_float_uint_1 = OpTypeArray %float %uint_1
+ %_struct_11 = OpTypeStruct %v4float %float %_arr_float_uint_1 %_arr_float_uint_1
+%_ptr_Output__struct_11 = OpTypePointer Output %_struct_11
+         %13 = OpVariable %_ptr_Output__struct_11 Output
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+%_ptr_Input_int = OpTypePointer Input %int
+%gl_VertexIndex = OpVariable %_ptr_Input_int Input
+       %bool = OpTypeBool
+   %float_n4 = OpConstant %float -4
+    %float_1 = OpConstant %float 1
+      %int_2 = OpConstant %int 2
+    %float_4 = OpConstant %float 4
+   %float_n1 = OpConstant %float -1
+    %float_0 = OpConstant %float 0
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+          %4 = OpFunction %void None %3
+          %5 = OpLabel
+         %18 = OpLoad %int %gl_VertexIndex
+         %20 = OpIEqual %bool %18 %int_0
+         %23 = OpSelect %float %20 %float_n4 %float_1
+         %26 = OpIEqual %bool %18 %int_2
+         %29 = OpSelect %float %26 %float_4 %float_n1
+         %31 = OpCompositeConstruct %v4float %23 %29 %float_0 %float_1
+         %33 = OpAccessChain %_ptr_Output_v4float %13 %int_0
+               OpStore %33 %31
+               OpReturn
+               OpFunctionEnd

naga/tests/out/glsl/quad-vert.main.Vertex.glsl

@@ -17,7 +17,7 @@ vec2 v_uv = vec2(0.0);
 
 vec2 a_uv_1 = vec2(0.0);
 
-gen_gl_PerVertex perVertexStruct = gen_gl_PerVertex(vec4(0.0, 0.0, 0.0, 1.0), 1.0, float[1](0.0), float[1](0.0));
+gen_gl_PerVertex unnamed = gen_gl_PerVertex(vec4(0.0, 0.0, 0.0, 1.0), 1.0, float[1](0.0), float[1](0.0));
 
 vec2 a_pos_1 = vec2(0.0);
 
@@ -29,7 +29,7 @@ void main_1() {
     vec2 _e6 = a_uv_1;
     v_uv = _e6;
     vec2 _e7 = a_pos_1;
-    perVertexStruct.gen_gl_Position = vec4(_e7.x, _e7.y, 0.0, 1.0);
+    unnamed.gen_gl_Position = vec4(_e7.x, _e7.y, 0.0, 1.0);
     return;
 }
 
@@ -40,7 +40,7 @@ void main() {
     a_pos_1 = a_pos;
     main_1();
     vec2 _e7 = v_uv;
-    vec4 _e8 = perVertexStruct.gen_gl_Position;
+    vec4 _e8 = unnamed.gen_gl_Position;
     type_4 _tmp_return = type_4(_e7, _e8);
     _vs2fs_location0 = _tmp_return.member;
     gl_Position = _tmp_return.gen_gl_Position;

naga/tests/out/glsl/unnamed-gl-per-vertex.main.Vertex.glsl

@@ -0,0 +1,34 @@
+#version 310 es
+
+precision highp float;
+precision highp int;
+
+struct type_4 {
+    vec4 member;
+    float member_1;
+    float member_2[1];
+    float member_3[1];
+};
+type_4 global = type_4(vec4(0.0, 0.0, 0.0, 1.0), 1.0, float[1](0.0), float[1](0.0));
+
+int global_1 = 0;
+
+
+void function() {
+    int _e9 = global_1;
+    global.member = vec4(((_e9 == 0) ? -4.0 : 1.0), ((_e9 == 2) ? 4.0 : -1.0), 0.0, 1.0);
+    return;
+}
+
+void main() {
+    uint param = uint(gl_VertexID);
+    global_1 = int(param);
+    function();
+    float _e6 = global.member.y;
+    global.member.y = -(_e6);
+    vec4 _e8 = global.member;
+    gl_Position = _e8;
+    gl_Position.yz = vec2(-gl_Position.y, gl_Position.z * 2.0 - gl_Position.w);
+    return;
+}
+

naga/tests/out/hlsl/quad-vert.hlsl

@@ -22,7 +22,7 @@ gl_PerVertex Constructgl_PerVertex(float4 arg0, float arg1, float arg2[1], float
 
 static float2 v_uv = (float2)0;
 static float2 a_uv_1 = (float2)0;
-static gl_PerVertex perVertexStruct = Constructgl_PerVertex(float4(0.0, 0.0, 0.0, 1.0), 1.0, (float[1])0, (float[1])0);
+static gl_PerVertex unnamed = Constructgl_PerVertex(float4(0.0, 0.0, 0.0, 1.0), 1.0, (float[1])0, (float[1])0);
 static float2 a_pos_1 = (float2)0;
 
 struct VertexOutput_main {
@@ -35,7 +35,7 @@ void main_1()
     float2 _expr6 = a_uv_1;
     v_uv = _expr6;
     float2 _expr7 = a_pos_1;
-    perVertexStruct.gl_Position = float4(_expr7.x, _expr7.y, 0.0, 1.0);
+    unnamed.gl_Position = float4(_expr7.x, _expr7.y, 0.0, 1.0);
     return;
 }
 
@@ -52,7 +52,7 @@ VertexOutput_main main(float2 a_uv : LOC1, float2 a_pos : LOC0)
     a_pos_1 = a_pos;
     main_1();
     float2 _expr7 = v_uv;
-    float4 _expr8 = perVertexStruct.gl_Position;
+    float4 _expr8 = unnamed.gl_Position;
     const type_4 type_4_ = Constructtype_4(_expr7, _expr8);
     const VertexOutput_main type_4_1 = { type_4_.member, type_4_.gl_Position };
     return type_4_1;

naga/tests/out/hlsl/unnamed-gl-per-vertex.hlsl

@@ -0,0 +1,36 @@
+struct type_4 {
+    float4 member : SV_Position;
+    float member_1;
+    float member_2[1];
+    float member_3[1];
+    int _end_pad_0;
+};
+
+type_4 Constructtype_4(float4 arg0, float arg1, float arg2[1], float arg3[1]) {
+    type_4 ret = (type_4)0;
+    ret.member = arg0;
+    ret.member_1 = arg1;
+    ret.member_2 = arg2;
+    ret.member_3 = arg3;
+    return ret;
+}
+
+static type_4 global = Constructtype_4(float4(0.0, 0.0, 0.0, 1.0), 1.0, (float[1])0, (float[1])0);
+static int global_1 = (int)0;
+
+void function()
+{
+    int _expr9 = global_1;
+    global.member = float4(((_expr9 == 0) ? -4.0 : 1.0), ((_expr9 == 2) ? 4.0 : -1.0), 0.0, 1.0);
+    return;
+}
+
+float4 main(uint param : SV_VertexID) : SV_Position
+{
+    global_1 = int(param);
+    function();
+    float _expr6 = global.member.y;
+    global.member.y = -(_expr6);
+    float4 _expr8 = global.member;
+    return _expr8;
+}

naga/tests/out/hlsl/unnamed-gl-per-vertex.ron

@@ -0,0 +1,12 @@
+(
+    vertex:[
+        (
+            entry_point:"main",
+            target_profile:"vs_5_1",
+        ),
+    ],
+    fragment:[
+    ],
+    compute:[
+    ],
+)