From: Jason Ekstrand <jason@jlekstrand.net>
Date: Sat, 30 May 2020 16:07:46 +0000 (-0500)
Subject: spirv/subgroups: Stop incrementing w
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=ea246c3950777d5c5dd5363b0cfc15215db12b11;p=mesa.git

spirv/subgroups: Stop incrementing w

The w++ is to handle a differences between the KHR extension and Vulkan
1.1 feature where the Vulkan 1.1 instructions take an scope parameter.
While incrementing w technically works, it's really subtle and very easy
to miss when reading the code.

Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5278>
---

diff --git a/src/compiler/spirv/vtn_subgroup.c b/src/compiler/spirv/vtn_subgroup.c
index 9ebc7c14f09..aa8ddff5654 100644
--- a/src/compiler/spirv/vtn_subgroup.c
+++ b/src/compiler/spirv/vtn_subgroup.c
@@ -88,13 +88,14 @@ vtn_handle_subgroup(struct vtn_builder *b, SpvOp opcode,
       break;
    }
 
-   case SpvOpGroupNonUniformBallot: ++w; /* fallthrough */
+   case SpvOpGroupNonUniformBallot:
    case SpvOpSubgroupBallotKHR: {
+      bool has_scope = (opcode != SpvOpSubgroupBallotKHR);
       vtn_fail_if(val->type->type != glsl_vector_type(GLSL_TYPE_UINT, 4),
                   "OpGroupNonUniformBallot must return a uvec4");
       nir_intrinsic_instr *ballot =
          nir_intrinsic_instr_create(b->nb.shader, nir_intrinsic_ballot);
-      ballot->src[0] = nir_src_for_ssa(vtn_get_nir_ssa(b, w[3]));
+      ballot->src[0] = nir_src_for_ssa(vtn_get_nir_ssa(b, w[3 + has_scope]));
       nir_ssa_dest_init(&ballot->instr, &ballot->dest, 4, 32, NULL);
       ballot->num_components = 4;
       nir_builder_instr_insert(&b->nb, &ballot->instr);
@@ -177,19 +178,24 @@ vtn_handle_subgroup(struct vtn_builder *b, SpvOp opcode,
       break;
    }
 
-   case SpvOpGroupNonUniformBroadcastFirst: ++w; /* fallthrough */
-   case SpvOpSubgroupFirstInvocationKHR:
+   case SpvOpGroupNonUniformBroadcastFirst:
+   case SpvOpSubgroupFirstInvocationKHR: {
+      bool has_scope = (opcode != SpvOpSubgroupFirstInvocationKHR);
       vtn_build_subgroup_instr(b, nir_intrinsic_read_first_invocation,
-                               val->ssa, vtn_ssa_value(b, w[3]), NULL, 0, 0);
+                               val->ssa, vtn_ssa_value(b, w[3 + has_scope]),
+                               NULL, 0, 0);
       break;
+   }
 
    case SpvOpGroupNonUniformBroadcast:
-   case SpvOpGroupBroadcast: ++w; /* fallthrough */
-   case SpvOpSubgroupReadInvocationKHR:
+   case SpvOpGroupBroadcast:
+   case SpvOpSubgroupReadInvocationKHR: {
+      bool has_scope = (opcode != SpvOpSubgroupReadInvocationKHR);
       vtn_build_subgroup_instr(b, nir_intrinsic_read_invocation,
-                               val->ssa, vtn_ssa_value(b, w[3]),
-                               vtn_get_nir_ssa(b, w[4]), 0, 0);
+                               val->ssa, vtn_ssa_value(b, w[3 + has_scope]),
+                               vtn_get_nir_ssa(b, w[4 + has_scope]), 0, 0);
       break;
+   }
 
    case SpvOpGroupNonUniformAll:
    case SpvOpGroupNonUniformAny: