From b479de8537ad34ec56d61f87d53a327a175eab36 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: --- src/compiler/spirv/vtn_variables.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) 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 -- 2.30.2