Mesa (staging/20.1): spirv: Don't emit RMW for vector indexing in shared or global

[GitLab Mirror]

Module: Mesa
Branch: staging/20.1
Commit: 9beee6ffe9d45ee06ec0c47bf9ad7ee81eb17b65
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9beee6ffe9d45ee06ec0c47bf9ad7ee81eb17b65

Author: Jason Ekstrand <jason at jlekstrand.net>
Date:   Mon Aug 17 10:51:32 2020 -0500

spirv: Don't emit RMW for vector indexing in shared or global

Anything that fails the is_external_block check is getting the
vtn_local_load/store path which does read-modify-write which isn't
correct if the variable mode can be written cross-workgroup.

Cc: mesa-stable at lists.freedesktop.org
Reviewed-by: Jesse Natalie <jenatali at microsoft.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6372>
(cherry picked from commit b479de8537ad34ec56d61f87d53a327a175eab36)

---

 .pick_status.json                  |  2 +-
 src/compiler/spirv/vtn_variables.c | 22 +++++++++++++++++-----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index ebed256ed17..65db8bcbff5 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -76,7 +76,7 @@
         "description": "spirv: Don't emit RMW for vector indexing in shared or global",
         "nominated": true,
         "nomination_type": 0,
-        "resolution": 0,
+        "resolution": 1,
         "master_sha": null,
         "because_sha": null
     },
diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c
index 93c4e004ab1..77cdcd56a58 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -131,6 +131,18 @@ vtn_mode_uses_ssa_offset(struct vtn_builder *b,
           mode == vtn_variable_mode_push_constant;
 }
 
+static bool
+vtn_mode_is_cross_invocation(struct vtn_builder *b,
+                             enum vtn_variable_mode mode)
+{
+   return mode == vtn_variable_mode_ssbo ||
+          mode == vtn_variable_mode_ubo ||
+          mode == vtn_variable_mode_phys_ssbo ||
+          mode == vtn_variable_mode_push_constant ||
+          mode == vtn_variable_mode_workgroup ||
+          mode == vtn_variable_mode_cross_workgroup;
+}
+
 static bool
 vtn_pointer_is_external_block(struct vtn_builder *b,
                               struct vtn_pointer *ptr)
@@ -1093,11 +1105,11 @@ _vtn_variable_load_store(struct vtn_builder *b, bool load,
       if (glsl_type_is_vector_or_scalar(ptr->type->type)) {
          /* We hit a vector or scalar; go ahead and emit the load[s] */
          nir_deref_instr *deref = vtn_pointer_to_deref(b, ptr);
-         if (vtn_pointer_is_external_block(b, ptr)) {
-            /* If it's external, we call nir_load/store_deref directly.  The
-             * vtn_local_load/store helpers are too clever and do magic to
-             * avoid array derefs of vectors.  That magic is both less
-             * efficient than the direct load/store and, in the case of
+         if (vtn_mode_is_cross_invocation(b, ptr->mode)) {
+            /* If it's cross-invocation, we call nir_load/store_deref
+             * directly.  The vtn_local_load/store helpers are too clever and
+             * do magic to avoid array derefs of vectors.  That magic is both
+             * less efficient than the direct load/store and, in the case of
              * stores, is broken because it creates a race condition if two
              * threads are writing to different components of the same vector
              * due to the load+insert+store it uses to emulate the array