icculus / mojoshader

Use Direct3D shaders with other 3D rendering APIs.
https://icculus.org/mojoshader/
zlib License
150 stars 37 forks source link

discard vs OpKill #34

Open kg opened 4 years ago

kg commented 4 years ago

discard in HLSL explicitly is specified as not terminating execution (https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/discard--sm4---asm-). In GLSL, it is apparently unspecified, and while NVIDIA continues execution AMD does not. In comparison, SPIR-V says that OpKill terminates execution of the fragment shader and it must be the end of a block.

The result of this is that right now if you have a SM3 shader that does 'discard' without immediately returning, mojoshader will produce a buggy or outright broken SPIRV shader that may cause a crash if validation is enabled or you run under renderdoc.

One easy way to do this by accident is to use 'clip' or 'discard' inside of a function. If the shader main() calls the function and the function discards, there's no way for the function performing the discard to 'return' out of the caller, and producing the required behavior (kill-then-return) might not be possible to do reliably at all unless you can be certain fxc is going to flatten everything and not put anything after the discard operation.

I don't know if this should be "fixed", and it's kind of weird semantically to have discard not terminate execution in SM3. But it's observable in SM4 and it is a "real" thing on hardware since other fragments in the execution group may not have discarded, so maybe this does matter. In my case I was able to find the problem shader pretty easily but it took some digging to figure out discard was the issue because the validation error I happened to get was something different entirely:

In hlsl and glsl, this shader is valid and does not contain UB, since execution continues after the discard and the output regs are initialized. In SPIRV the output regs are not initialized since the discard killed execution and then you get a validation failure due to the uninitialized outputs.

krolli commented 4 years ago

As mentioned on discord, one possible solution would be to use SPV_EXT_demote_to_helper_invocation if supported. It would also require checking support and enabling VK_EXT_shader_demote_to_helper_invocation in Vulkan. I'm not sure whether there is similar extension for OpenGL, but since there is always GLSL as an option, it probably isn't that big a deal.

flibitijibibo commented 4 years ago

The OpenGL extension is simply GL_EXT_demote_to_helper_invocation. The extension is available on all drivers made after ~summer 2019, so it would be a reasonable extension to add if someone wants to implement it in MojoShader and FNA3D.