From b6a1444ed1bba9acb6dd84031d17b708b6ead604 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Mon, 17 Aug 2020 10:51:32 -0500 Subject: [PATCH] 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@lists.freedesktop.org Reviewed-by: Jesse Natalie Part-of: (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 453ae16b2f3..dd25d045d31 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1651,7 +1651,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 47932f0cd16..1cb848a3af8 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -124,6 +124,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) @@ -1074,11 +1086,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