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

[Jason Ekstrand]

On Tue, Sep 5, 2017 at 8:33 AM, Connor Abbott <cwabbott0 at gmail.com> wrote:

> 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).
>

Thanks for pointing that out!  Dave, Bas, would either of you mind getting
those properly hooked up?  As a side-note, this patch also made me consider
re-working the NIR barrier intrinsics to be a bit more SPIR-V like.

--Jason


> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170905/057cbc30/attachment-0001.html>