[Mesa-dev] [PATCH 42/44] spirv: Rework barriers

[Connor Abbott]

As a quick drive-by, yeah, I noticed this too, and it's going to
require fixes to radv to not break things since none of the other NIR
opcodes are hooked up (this will be needed for the NIR path in
radeonsi too, since GLSL-to-NIR already uses those opcodes).

On Tue, Sep 5, 2017 at 11:13 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> Our previous handling of barriers always used the big hammer and didn't
> correctly emit memory barriers when specified along with a control
> barrier.  This commit completely reworks the way we emit barriers to
> make things both more precise and more correct.
> ---
>  src/compiler/spirv/spirv_to_nir.c | 132 ++++++++++++++++++++++++++++++++------
>  1 file changed, 114 insertions(+), 18 deletions(-)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c
> index 8653685..6fb27cb 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -2570,36 +2570,132 @@ vtn_handle_composite(struct vtn_builder *b, SpvOp opcode,
>  }
>
>  static void
> +vtn_emit_barrier(struct vtn_builder *b, nir_intrinsic_op op)
> +{
> +   nir_intrinsic_instr *intrin = nir_intrinsic_instr_create(b->shader, op);
> +   nir_builder_instr_insert(&b->nb, &intrin->instr);
> +}
> +
> +static void
> +vtn_emit_memory_barrier(struct vtn_builder *b, SpvScope scope,
> +                        SpvMemorySemanticsMask semantics)
> +{
> +   static const SpvMemorySemanticsMask all_memory_semantics =
> +      SpvMemorySemanticsUniformMemoryMask |
> +      SpvMemorySemanticsWorkgroupMemoryMask |
> +      SpvMemorySemanticsAtomicCounterMemoryMask |
> +      SpvMemorySemanticsImageMemoryMask;
> +
> +   /* If we're not actually doing a memory barrier, bail */
> +   if (!(semantics & all_memory_semantics))
> +      return;
> +
> +   /* GL and Vulkan don't have these */
> +   assert(scope != SpvScopeCrossDevice);
> +
> +   if (scope == SpvScopeSubgroup)
> +      return; /* Nothing to do here */
> +
> +   if (scope == SpvScopeWorkgroup) {
> +      vtn_emit_barrier(b, nir_intrinsic_group_memory_barrier);
> +      return;
> +   }
> +
> +   /* There's only two scopes thing left */
> +   assert(scope == SpvScopeInvocation || scope == SpvScopeDevice);
> +
> +   if ((semantics & all_memory_semantics) == all_memory_semantics) {
> +      vtn_emit_barrier(b, nir_intrinsic_memory_barrier);
> +      return;
> +   }
> +
> +   /* Issue a bunch of more specific barriers */
> +   uint32_t bits = semantics;
> +   while (bits) {
> +      SpvMemorySemanticsMask semantic = 1 << u_bit_scan(&bits);
> +      switch (semantic) {
> +      case SpvMemorySemanticsUniformMemoryMask:
> +         vtn_emit_barrier(b, nir_intrinsic_memory_barrier_buffer);
> +         break;
> +      case SpvMemorySemanticsWorkgroupMemoryMask:
> +         vtn_emit_barrier(b, nir_intrinsic_memory_barrier_shared);
> +         break;
> +      case SpvMemorySemanticsAtomicCounterMemoryMask:
> +         vtn_emit_barrier(b, nir_intrinsic_memory_barrier_atomic_counter);
> +         break;
> +      case SpvMemorySemanticsImageMemoryMask:
> +         vtn_emit_barrier(b, nir_intrinsic_memory_barrier_image);
> +         break;
> +      default:
> +         break;;
> +      }
> +   }
> +}
> +
> +static void
>  vtn_handle_barrier(struct vtn_builder *b, SpvOp opcode,
>                     const uint32_t *w, unsigned count)
>  {
> -   nir_intrinsic_op intrinsic_op;
>     switch (opcode) {
>     case SpvOpEmitVertex:
>     case SpvOpEmitStreamVertex:
> -      intrinsic_op = nir_intrinsic_emit_vertex;
> -      break;
>     case SpvOpEndPrimitive:
> -   case SpvOpEndStreamPrimitive:
> -      intrinsic_op = nir_intrinsic_end_primitive;
> -      break;
> -   case SpvOpMemoryBarrier:
> -      intrinsic_op = nir_intrinsic_memory_barrier;
> -      break;
> -   case SpvOpControlBarrier:
> -      intrinsic_op = nir_intrinsic_barrier;
> +   case SpvOpEndStreamPrimitive: {
> +      nir_intrinsic_op intrinsic_op;
> +      switch (opcode) {
> +      case SpvOpEmitVertex:
> +      case SpvOpEmitStreamVertex:
> +         intrinsic_op = nir_intrinsic_emit_vertex;
> +         break;
> +      case SpvOpEndPrimitive:
> +      case SpvOpEndStreamPrimitive:
> +         intrinsic_op = nir_intrinsic_end_primitive;
> +         break;
> +      default:
> +         unreachable("Invalid opcode");
> +      }
> +
> +      nir_intrinsic_instr *intrin =
> +         nir_intrinsic_instr_create(b->shader, intrinsic_op);
> +
> +      switch (opcode) {
> +      case SpvOpEmitStreamVertex:
> +      case SpvOpEndStreamPrimitive:
> +         nir_intrinsic_set_stream_id(intrin, w[1]);
> +         break;
> +      default:
> +         break;
> +      }
> +
> +      nir_builder_instr_insert(&b->nb, &intrin->instr);
>        break;
> -   default:
> -      unreachable("unknown barrier instruction");
>     }
>
> -   nir_intrinsic_instr *intrin =
> -      nir_intrinsic_instr_create(b->shader, intrinsic_op);
> +   case SpvOpMemoryBarrier: {
> +      SpvScope scope = vtn_constant_value(b, w[1])->values[0].u32[0];
> +      SpvMemorySemanticsMask semantics =
> +         vtn_constant_value(b, w[2])->values[0].u32[0];
> +      vtn_emit_memory_barrier(b, scope, semantics);
> +      return;
> +   }
> +
> +   case SpvOpControlBarrier: {
> +      SpvScope execution_scope =
> +         vtn_constant_value(b, w[1])->values[0].u32[0];
> +      if (execution_scope == SpvScopeWorkgroup)
> +         vtn_emit_barrier(b, nir_intrinsic_barrier);
>
> -   if (opcode == SpvOpEmitStreamVertex || opcode == SpvOpEndStreamPrimitive)
> -      nir_intrinsic_set_stream_id(intrin, w[1]);
> +      SpvScope memory_scope =
> +         vtn_constant_value(b, w[2])->values[0].u32[0];
> +      SpvMemorySemanticsMask memory_semantics =
> +         vtn_constant_value(b, w[3])->values[0].u32[0];
> +      vtn_emit_memory_barrier(b, memory_scope, memory_semantics);
> +      break;
> +   }
>
> -   nir_builder_instr_insert(&b->nb, &intrin->instr);
> +   default:
> +      unreachable("unknown barrier instruction");
> +   }
>  }
>
>  static unsigned
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev