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

glslang produces SPIR-V that the Vulkan validation layer doesn't like (DepthReplacing related) #1418

Closed
hrydgard opened this issue Jun 26, 2018 · 7 comments
Labels

Comments

@hrydgard
Copy link
Contributor

When creating a shader module using some SPIR-V produced by glslang, I get the following error:

ERROR: [Validation] Code 5 : Object: VK_NULL_HANDLE (Type = 0) | SPIR-V module not valid: Vulkan spec requires DepthReplacing execution mode to be declared when using BuiltIn FragDepth. ID <0> (OpStore) is referencing ID <8> (OpVariable) which is decorated with BuiltIn FragDepth in function <4>.

The SPIR-V was produced from the following GLSL:

#version 450
#extension GL_ARB_separate_shader_objects : enable
#extension GL_ARB_shading_language_420pack : enable
#extension GL_ARB_conservative_depth : enable
layout (depth_unchanged) out float gl_FragDepth;
layout (binding = 0) uniform sampler2D tex;
layout (push_constant) uniform params {
	int u_stencilValue;
};
layout (location = 0) in vec2 v_texcoord0;
layout (location = 0) out vec4 fragColor0;

void main() {
	gl_FragDepth = gl_FragCoord.z;

	if (u_stencilValue == 0) {
		fragColor0 = vec4(0.0);
	} else {
		vec4 index = texture(tex, v_texcoord0);
		int indexBits = int(floor(index.a * 255.99)) & 0xFF;
		if ((indexBits & u_stencilValue) == 0)
			discard;
		fragColor0 = index.aaaa;
	}
}

Note that we update the depth with itself to prod some buggy mobile drivers that ignore OpKill (discard) unless the shader writes to depth, for whatever reason.

For the curious, this is used to write an image to stencil, a bitplane at a time.

@hrydgard hrydgard changed the title glslang produces SPIR-V that the Vulkan validation layer doesn't like glslang produces SPIR-V that the Vulkan validation layer doesn't like (DepthReplacing related) Jun 26, 2018
@johnkslang
Copy link
Member

But, you declared that depth does not change, so the SPIR-V does not declare that it does. From your shader:

layout (depth_unchanged) out float gl_FragDepth;

@johnkslang
Copy link
Member

Ah, I see. That means it is a validator bug, as the validator cannot know whether or not depth is changed, so it cannot say that this should have been declared.

@johnkslang
Copy link
Member

I suspect this sentence in the Vulkan spec. slightly over states things:

To write to FragDepth, a shader must declare the DepthReplacing execution mode.

It should probably say something more like:

To modify FragDepth and have the modification replace the pipeline depth...

@johnkslang
Copy link
Member

I've taken this inside Khronos. There is an alternative that both DepthReplacing and DepthUnchanged should have been declared. https://gitlab.khronos.org/vulkan/vulkan/merge_requests/2741

@unknownbrackets
Copy link

unknownbrackets commented Jul 9, 2018

That was my original impression from reading the spec - that it should have both.

Note the GL extension:

When the layout qualifier is <depth_unchanged>, the shader compiler will honor
any modification to gl_FragDepth, but the rest of the GL assume that
gl_FragDepth is not assigned a new value.

And this related issue:
https://www.khronos.org/bugzilla/show_bug.cgi?id=726

Which implies that there was some use case that motivated more strictly defining behavior here. Not sure what it was. I can only think of code reuse and influencing early z as use cases for depth_unchanged.

The bug being worked around is documented here, in case useful:
https://developer.qualcomm.com/forum/qdn-forums/software/adreno-gpu-sdk/35916

-[Unknown]

@hrydgard
Copy link
Contributor Author

glslang + validation layer seems to cooperate better now, not getting bad validation errors anymore at least.

@arcady-lunarg
Copy link
Contributor

Closing this as resolved, per the last comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants