Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spriv-cross no longer generates "vertex void" calls for vertex shaders with no outputs. #2261

Open
alecazam opened this issue Jan 11, 2024 · 4 comments
Labels
question Further progress depends on answer from issue creator.

Comments

@alecazam
Copy link

This is a spirv-cross issue. The older version (6/20) did the correct transpile to "vertex void". The new version always generats "vertex vsmain_out". vsmain_out contains a gl_Position, but that is never initialized, and then just return from the vertex shader. Then that's enough to freak out Metal trying to run this with rasterization disabled. We have a VS with no PS, and set Metal's RasterizationEnabled to false.

This is the error returned by Metal.

Assert error: Renderer Crash, Failed to create render pipeline for shader Test: RasterizationEnabled is false but the vertex shader's return type is not void

I'd reported this here, but then found this thread. Is there something we can set to get this behavior back like in the old spriv-cross.
KhronosGroup/SPIRV-Tools#5525

The correct test for this is a shader that never uses gl_Position in the GLSL body, and so should not generate any output. We write directly into the SSBO buffer. The code above is not the correct test case, since it has a VS and PS and is using gl_Position. Hopefully this is enough to demonstrate a shader without using rasterization and gl_position.

Test.vert.glsl, we're using 450core as the source version type.

layout( binding = 0, set = DescriptorSet ) restrict buffer OcclusionResult {
	uint o_isOccluded[];
};

void main()
{
   o_isOccluded[0] = 1;
}
@alecazam
Copy link
Author

My workaround for now, is to use new glslc and new spriv-opt, and the old spirv-cross for MSL codegen. But I'd like to use a spirv-cross newer than 6/20.

@alecazam
Copy link
Author

alecazam commented Jan 11, 2024

I'm trying to isolate all this, bug we also have this. This isn't referenced in the code, and the old spriv-cross detected this wasn't used, and kept the "vertex void". The new one adds this in, and then produces an uninitialized gl_Position.

So I'm going to add a define to strip this code, but seems like this should be detected as unused and not change the VS signature.

        out gl_PerVertex
	{
		// Need pos to be same for FPZ and occlusion, avoid reordering math
		invariant float4 gl_Position;
		float gl_PointSize; // always square
	};

Indeed that fixed the issue. So we have a workaround for this. But this and other outputs may be unreferenced and not stripped from the output and cause issue.

@HansKristian-Work
Copy link
Contributor

Do you have a concrete SPIR-V reproducer? I tried compiling that GLSL (added #version 450 and replaced DescriptorSet to 0), got

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 11
; Bound: 16
; Schema: 0
               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Vertex %main "main"
               OpSource GLSL 450
               OpName %main "main"
               OpName %OcclusionResult "OcclusionResult"
               OpMemberName %OcclusionResult 0 "o_isOccluded"
               OpName %_ ""
               OpDecorate %_runtimearr_uint ArrayStride 4
               OpMemberDecorate %OcclusionResult 0 Restrict
               OpMemberDecorate %OcclusionResult 0 Offset 0
               OpDecorate %OcclusionResult BufferBlock
               OpDecorate %_ DescriptorSet 0
               OpDecorate %_ Binding 0
       %void = OpTypeVoid
          %3 = OpTypeFunction %void
       %uint = OpTypeInt 32 0
%_runtimearr_uint = OpTypeRuntimeArray %uint
%OcclusionResult = OpTypeStruct %_runtimearr_uint
%_ptr_Uniform_OcclusionResult = OpTypePointer Uniform %OcclusionResult
          %_ = OpVariable %_ptr_Uniform_OcclusionResult Uniform
        %int = OpTypeInt 32 1
      %int_0 = OpConstant %int 0
     %uint_1 = OpConstant %uint 1
%_ptr_Uniform_uint = OpTypePointer Uniform %uint
       %main = OpFunction %void None %3
          %5 = OpLabel
         %15 = OpAccessChain %_ptr_Uniform_uint %_ %int_0 %int_0
               OpStore %15 %uint_1
               OpReturn
               OpFunctionEnd

./spirv-cross --msl /tmp/test.spv

#include <metal_stdlib>
#include <simd/simd.h>

using namespace metal;

struct OcclusionResult
{
    uint o_isOccluded[1];
};

vertex void main0(device OcclusionResult& _10 [[buffer(0)]])
{
    _10.o_isOccluded[0] = 1u;
}

@HansKristian-Work HansKristian-Work added the question Further progress depends on answer from issue creator. label Jan 23, 2024
@alecazam
Copy link
Author

alecazam commented Feb 7, 2024

I will try to isolate this code and send a repro. But it may be a few weeks. We did have code setting point size, that that was in the same struct. But #ifdef'ing out the struct fixed our return values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further progress depends on answer from issue creator.
Projects
None yet
Development

No branches or pull requests

2 participants