spirv: fix emitting switch cases that directly jump to the merge block
authorSamuel Pitoiset <samuel.pitoiset@gmail.com>
Thu, 3 Sep 2020 20:02:01 +0000 (22:02 +0200)
committerMarge Bot <eric+marge@anholt.net>
Fri, 4 Sep 2020 21:34:47 +0000 (21:34 +0000)
As shown in the valid SPIR-V below, if one switch case statement
directly jumps to the merge block, it has no branches at all and
we have to reset the fall variable. Otherwise, it creates an
unintentional fallthrough.

       OpSelectionMerge %97 None
       OpSwitch %96 %97 1 %99 2 %100
%100 = OpLabel
%102 = OpAccessChain %_ptr_StorageBuffer_v4float %86 %uint_0 %uint_37
%103 = OpLoad %v4float %102
%104 = OpBitcast %v4uint %103
%105 = OpCompositeExtract %uint %104 0
%106 = OpShiftLeftLogical %uint %105 %uint_1
       OpBranch %97
 %99 = OpLabel
       OpBranch %97
 %97 = OpLabel
%107 = OpPhi %uint %uint_4 %75 %uint_5 %99 %106 %100

This fixes serious corruption in Horizon Zero Dawn.

v2: Changed the code to skip the entire if-block instead of resetting
    the fallthrough variable.

Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3460
Cc: mesa-stable
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Reviewed-by: Daniel Schürmann <daniel@schuermann.dev>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6590>

src/compiler/spirv/vtn_cfg.c

index 83ca13b734cb3448595852db35fd081846626a15..a1573565b81eb6c13275c0fd76efdfe023370c50 100644 (file)
@@ -1165,6 +1165,13 @@ vtn_emit_cf_list_structured(struct vtn_builder *b, struct list_head *cf_list,
          vtn_foreach_cf_node(case_node, &vtn_switch->cases) {
             struct vtn_case *cse = vtn_cf_node_as_case(case_node);
 
          vtn_foreach_cf_node(case_node, &vtn_switch->cases) {
             struct vtn_case *cse = vtn_cf_node_as_case(case_node);
 
+            /* If this case jumps directly to the break block, we don't have
+             * to handle the case as the body is empty and doesn't fall
+             * through.
+             */
+            if (cse->block == vtn_switch->break_block)
+               continue;
+
             /* Figure out the condition */
             nir_ssa_def *cond =
                vtn_switch_case_condition(b, vtn_switch, sel, cse);
             /* Figure out the condition */
             nir_ssa_def *cond =
                vtn_switch_case_condition(b, vtn_switch, sel, cse);