search

microsoft / DirectXShaderCompiler

This repo hosts the source for the DirectX Shader Compiler which is based on LLVM/Clang.
Other
3.04k stars 678 forks source link

[SPIR-V] Provably dead code is still compiled when targeting SPIRV #6788

Closed SteveUrquhart closed 1 month ago

SteveUrquhart commented 1 month ago

Description When targeting SPIRV, dxc suffers from a lack of early dead-code elimination. Provably false code is still compiled, while targeting dxil avoids this problem. This discrepancy in behavior is especially problematic for large HLSL codebases.

Steps to Reproduce Consider the shader and parameters shown in the following link: https://godbolt.org/z/j9jK35j16. Although line 19 is illegal, we see that compilation for dxil succeeds. The user may edit line 31 to set uint choice to 1, and see that dxil does consider line 19 to be illegal. After resetting line 31 to 0, the user may add "-spirv" after "-T vs_6_0 -E VSMain" in order to see the problem. Targeting SPIRV displays compilation of provably false branches.

Actual Behavior 19:16: error: sampling with implicit lod is only allowed in fragment and compute shaders return tex.Sample(samp, uv);

Environment dxc 1.8.2405

s-perron commented 1 month ago

This is a difficult issues to fix and still give a meaningful error message. My team will not be working on this. We will also need to figure out what it the long term direction for these types of errors.

DXIL waits until after optimization to issue the error. This is done during their validation phase.

On the SPIR-V side, we generally view a spir-val error after optimization as a bug in the compiler. We want to catch errors before the initial spir-v is generated because the spirv-val errors have generally lost the HLSL context.

In this case, if I make two changes: remove the error in DXC and turn off the "remove invalid opcode pass", which I want to do. The error message from spirv-val is something like:

fatal error: generated SPIR-V is invalid: OpEntryPoint Entry Point <id> '1[%VSMain]'s callgraph contains function <id> '57[%_FireTextureFetch]', which cannot be used with the current execution model:
ImplicitLod instructions require Fragment or GLCompute execution model: ImageSampleImplicitLod

  %_FireTextureFetch = OpFunction %v4float None %61

note: please file a bug report on https://github.com/Microsoft/DirectXShaderCompiler/issues with source code if possible

Note that spirv-val does not report the file and line number of the error in the HLSL source. Also, the error is reported in terms of the spir-v opcodes, and not the HLSL functions. This is a recipe for people getting errors they have no idea how to fix, especially in very large code bases.

If we were to fix this, we would have to

This would get the Spirv behaviour to match DXIL. The second step is potentially a lot of work.

SteveUrquhart commented 1 month ago

Thank you for the insightful comment!

s-perron commented 1 month ago

I think this is more work, and I don't think anyone will get to it. When HLSL compilation is available in clang, this should not be a problem. I will close the issue.