vkd3d-shader/ir: Remove dead code during normalisation.
Merge request reports
Activity
I think the backend should reject shaders with instructions outside blocks, except perhaps
nop
. A valid vsir shader should also contain no instructions outside blocks. Validating that in TPF is too complicated, but this touches on an issue which will arise in DXIL when branch and switch instructions are added. Using the same vsir instructions for both SM4/5 and SM6 shaders would require handling the SM4/5 structured control flow instructions in a normalisation stage, which would perform all validations.Hmmm, in my case the VSIR shader passed to the SPIR-V backend is valid and has no instructions outside a block. However, when dealing with unconditional control flow (e.g. a
return
orbreak
) inside a loop, the SPIR-V generator outputs, say,OpReturn
, which terminates the current block, but then goes on outputting any other dead code which appears after thatreturn
orbreak
without opening another block. The generated SPIR-V is thus illegal.I changed it so that after
OpReturn
another dummy SPIR-V block is began and the generated SPIR-V is valid. That's a problem with the SPIR-V code generation, not with VSIR.No, that comes directly from VSIR. Here is an HLSL example:
float4 main(float4 x : COLOR) : SV_TARGET { float4 ret = 0.0; for (;;) { return ret; ret = 1.0; } return ret; }
This compiles (with our HLSL compiler) to this TPF code:
ps_4_0 dcl_output o0.xyzw dcl_temps 4 mov r0.x, l(0.00000000e+00) mov r1.xyzw, l(0.00000000e+00, 0.00000000e+00, 0.00000000e+00, 0.00000000e+00) loop mov r2.xyzw, r1.xyzw mov r3.xyzw, r2.xyzw mov r0.x, l(-nan) break mov r1.xyzw, l(1.00000000e+00, 1.00000000e+00, 1.00000000e+00, 1.00000000e+00) endloop mov r0.x, r0.x not r0.x, r0.x if_nz r0.x mov r0.xyzw, r1.xyzw mov r3.xyzw, r0.xyzw endif mov r0.xyzw, r3.xyzw mov o0.xyzw, r0.xyzw ret
This is valid TPF, but notice that there is some dead code after the
break
. After compiling to SPIR-V that code will still be present, but without a block:; SPIR-V ; Version: 1.0 ; Generator: Wine VKD3D Shader Compiler; 8 ; Bound: 44 ; Schema: 0 OpCapability Shader OpMemoryModel Logical GLSL450 OpEntryPoint Fragment %main "main" %o0 OpExecutionMode %main OriginUpperLeft OpName %main "main" OpName %r0 "r0" OpName %r1 "r1" OpName %r2 "r2" OpName %r3 "r3" OpName %o0 "o0" OpName %loop0_header "loop0_header" OpName %loop0_body "loop0_body" OpName %loop0_continue "loop0_continue" OpName %loop0_merge "loop0_merge" OpName %branch0_merge "branch0_merge" OpName %branch0_true "branch0_true" OpDecorate %o0 Location 0 %void = OpTypeVoid %3 = OpTypeFunction %void %float = OpTypeFloat 32 %v4float = OpTypeVector %float 4 %_ptr_Private_v4float = OpTypePointer Private %v4float %r0 = OpVariable %_ptr_Private_v4float Private %r1 = OpVariable %_ptr_Private_v4float Private %r2 = OpVariable %_ptr_Private_v4float Private %r3 = OpVariable %_ptr_Private_v4float Private %_ptr_Output_v4float = OpTypePointer Output %v4float %o0 = OpVariable %_ptr_Output_v4float Output %float_0 = OpConstant %float 0 %_ptr_Private_float = OpTypePointer Private %float %uint = OpTypeInt 32 0 %uint_0 = OpConstant %uint 0 %19 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0 %float_n0x1_fffffep_128 = OpConstant %float -0x1.fffffep+128 %float_1 = OpConstant %float 1 %27 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1 %bool = OpTypeBool %main = OpFunction %void None %3 %4 = OpLabel %18 = OpInBoundsAccessChain %_ptr_Private_float %r0 %uint_0 OpStore %18 %float_0 OpStore %r1 %19 OpBranch %loop0_header %loop0_header = OpLabel OpLoopMerge %loop0_merge %loop0_continue None OpBranch %loop0_body %loop0_body = OpLabel OpCopyMemory %r2 %r1 OpCopyMemory %r3 %r2 %25 = OpInBoundsAccessChain %_ptr_Private_float %r0 %uint_0 OpStore %25 %float_n0x1_fffffep_128 OpBranch %loop0_merge OpStore %r1 %27 %loop0_continue = OpLabel OpBranch %loop0_header %loop0_merge = OpLabel %28 = OpInBoundsAccessChain %_ptr_Private_float %r0 %uint_0 %29 = OpLoad %float %28 %30 = OpInBoundsAccessChain %_ptr_Private_float %r0 %uint_0 OpStore %30 %29 %31 = OpInBoundsAccessChain %_ptr_Private_float %r0 %uint_0 %32 = OpLoad %float %31 %33 = OpBitcast %uint %32 %34 = OpNot %uint %33 %35 = OpBitcast %float %34 %36 = OpInBoundsAccessChain %_ptr_Private_float %r0 %uint_0 OpStore %36 %35 %37 = OpInBoundsAccessChain %_ptr_Private_float %r0 %uint_0 %38 = OpLoad %float %37 %39 = OpBitcast %uint %38 %41 = OpINotEqual %bool %39 %uint_0 OpSelectionMerge %branch0_merge None OpBranchConditional %41 %branch0_true %branch0_merge %branch0_true = OpLabel OpCopyMemory %r0 %r1 OpCopyMemory %r3 %r0 OpBranch %branch0_merge %branch0_merge = OpLabel OpCopyMemory %r0 %r3 OpCopyMemory %o0 %r0 OpReturn OpFunctionEnd
This is not allowed by SPIR-V, which is what my MR is trying to fix:
error: line 53: Store must appear in a block OpStore %r1 %27
However I just noticed that even with this MR there is still a validation problem:
error: line 56: Block '26[%skip0]' branches to the loop continue target '22[%loop0_continue]', but is not contained in the associated loop construct '20[%loop0_header]' OpBranch %loop0_continue
I thought that skipped blocks, being unreachable, were exempted from structure rules, but maybe I was mistaken. I'll check it better. Ignoring this last hiccup, that should explain why the MR is needed, even if with some additional tweak.
That's another approach. It's a bit more complicated because it requires more coordination with the rest of the code generator, which needs to know when to stop and resume emitting code, but at the same time still has to emit correct SPIR-V (it's not clear to me, say, if just stopping writing bytecode when you're not in a block is the right thing: as far as I know there could be other types of SPIR-V bytecode, such as annotations or I don't know what, that we still want to be generated). So my solution felt easier.
At the same time, my easier solution doesn't look too wrong: at least for the moment the SPIR-V generator is very crude, it receives VSIR instructions and directly emit the corresponding SPIR-V code without optimization. So if it receives dead code it is not inappropriate to emit other dead code: we just have to ensure that the SPIR-V code is valid, which is what this patch does.
added 1 commit
- 30424d38 - vkd3d-shader/ir: Remove dead code during normalisation.
As suggested by Henri, the latest revision doesn't touch the SPIR-V generator, but removes dead code (or, rather, sets it to NOP, but that's enough) as a normalisation pass. Now my (admittedly simple) tests pass SPIR-V validation and the two relevant shader runner tests pass on macOS.
added 1 commit
- d4ceaa38 - vkd3d-shader/ir: Remove dead code during normalisation.
added 1 commit
- b9c79f40 - vkd3d-shader/ir: Remove dead code during normalisation.
added 64 commits
-
b9c79f40...fcda20a8 - 63 commits from branch
wine:master
- d9c8b49e - vkd3d-shader/ir: Remove dead code during normalisation.
-
b9c79f40...fcda20a8 - 63 commits from branch