[naga/spv-in] Naga seems unable to consume any simple vertex shader from glslc due to gl_PerVertex attributes.

[alister-chowdhury]

Description
There seems to be something off when attempting to feed a simple vertex shader into naga, due to unused clip distance being present.

Repro steps

fullscreen.vert:

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

Building:

glslc -O fullscreen.vert -o fullscreen.spv
naga fullscreen.spv fullscreen.wgsl

And the resulting error:

Entry point main at Vertex is invalid:
        Capability Capabilities(CLIP_DISTANCE) is not supported
Generating wgsl output requires validation to succeed, and it failed in a previous step

For reference here is the spirv-dis:

; 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

There is a rather inconvenient workaround, which is to spirv-cross the spv back into glsl and feed that in.

Platform
On x64 Win10, using the latest naga and vulkansdk.

naga version: 0.14.0

glslc version:

shaderc v2023.7 v2023.7
spirv-tools v2023.5 v2022.4-352-g360d469b
glslang 11.1.0-803-g48f9ed8b

Target: SPIR-V 1.0

[fornwall]

The SPIR-V file contains OpMemberDecorate %_struct_11 2 BuiltIn ClipDistance without OpCapability ClipDistance, which is ok as long as the member is not read or written - KhronosGroup/glslang#472:

We had a big discussion about this internally to Khronos a while back and the decision was that it is okay to decorate with ClipDistance (and similar), but it's the actual reading and writing that needs the capability. Unless you read or wrote to clip distance, you are not really using the hardware feature, and the hardware capability is not in use, so the capability does not need to be declared, and the consumer should not care. The reasons are somewhat complex, but have to do with cross-stage linking and some other things.

Related: KhronosGroup/SPIRV-Tools#365

[eddyb]

The problem here is the use of glslc instead of glslangValidator: glslc produces less debuginfo by default, which breaks Naga's (arguably fragile) detection of gl_PerVertex:

wgpu/naga/src/front/spv/function.rs

Lines 375 to 383 in 4675737

	// Cull problematic builtins of gl_PerVertex.
	// See the docs for `Frontend::gl_per_vertex_builtin_access`.
	{
	let ty = &module.types[result.ty];
	match ty.inner {
	crate::TypeInner::Struct {
	members: ref original_members,
	span,
	} if ty.name.as_deref() == Some("gl_PerVertex") => {

I can't get glslc to generate exactly the same annotations as glslangValidator (-g does too much debuginfo, and Naga doesn't support it).

For the record, this works: glslangValidator -V fullscreen.vert -o fullscreen.spv.

(I wasn't even aware that the shaderc project made a glslc command, and have only ever used glslangValidator as the glslang CLI, since I never knew of any others)

[eddyb]

Found the original addition of that logic:

• Skip gl_PerVertex unused builtins in the SPIR-V frontend naga#2272

IMO it should be checking for a %Foo = OpTypeStruct ... with:

• one OpDecorate %Foo Block
• any OpMemberDecorate %Foo _ BuiltIn ...

As per the Vulkan spec that should uniquely identify the "builtin interface block" used by some entry-point).

Hopefully that's enough to allow ignoring names completely (cc @teoxoy @jimblandy).

[alister-chowdhury]

@eddyb

glslc has been around since vk1.0, and is referenced as the preferred compiler (of the author) in the Vulkan Tutorial.

I'd wager I'm not alone in treating it as the defacto standard for glsl -> spirv, although to be fair glslangValidator is referenced quite a lot aswell.

[eddyb]

glslc has been around since vk1.0

It's possible I missed it because I've been ignoring shaderc as a "weird library wrapper" (and didn't notice it even has binaries), whereas glslangValidator is the binary in the glslang component of the Vulkan SDK, so it feels "more official".

Anyway, that was there just to explain why I wouldn't have produced SPIR-V that causes Naga problems, and this may hold true for others as well.

And I do agree Naga should support all ways to define those builtins, if it has special logic for any of them. Btw, if anyone wants to rename the issue, mentioning gl_PerVertex builtins and glslc would be useful (otherwise this issue is deeply confusing, I was literally staring at meaningless spirv-dis diffs, having accidentally stumbled onto "glslangValidator output works in Naga but glslc doesn't" and decided to search the repo for literal mentions of gl_PerVertex because nothing else made sense).

[alister-chowdhury]

Yeah, I can understand that.

Since Vulkans inception, it's been a lot less like a product from a single group.

The tooling and whatknot came from a bunch of different people and the ecosystem sort of emerged, the VulkanSDK does a reasonable job of collecting them into one place in a ready to use state.

Giving you spirv-cross, spirv-reflect, dxc, glslc plus more as binaries and linkable libraries along with the actual core fundamental vulkan stuff in one place.

Good times, anyway I digress, I hope this title is more useful.

[Imberflur]

While attempting to implement the solution suggested here #4915 (comment), I stumbled onto this piece of code that was added in gfx-rs/naga#789:

wgpu/naga/src/front/spv/mod.rs

Lines 5130 to 5135 in 84ba4e5

	// 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())

I wonder if just checking for this name in addition to "gl_PerVertex" would be reliable enough?

[teoxoy]

I think we can just remove the ifs that are checking the name is gl_PerVertex. I was too conservative in gfx-rs/naga#2272, I don't think they are necessary.