[Mesa-dev] [PATCH 3/3] i965/nir/fs: Implement new barrier functions for compute shaders

[Francisco Jerez]

Jordan Justen <jordan.l.justen at intel.com> writes:

> For these nir intrinsics, we emit the same code as
> nir_intrinsic_memory_barrier:
>
>  * nir_intrinsic_memory_barrier_atomic_counter
>  * nir_intrinsic_memory_barrier_buffer
>  * nir_intrinsic_memory_barrier_image
>
> We treat these nir intrinsics as no-ops:
>
>  * nir_intrinsic_group_memory_barrier
>  * nir_intrinsic_memory_barrier_shared

I believe that a no-op is a valid implementation of these in practice,
as long as the following three conditions hold:

 - The compiler back-end won't reorder memory writes and reads
   afterwards.

 - All threads within a single thread group fit in a single subslice and
   therefore talk to the same HDC shared unit what AFAIK guarantees
   ordering and coherency -- I think this is always the case currently
   (AFAIK we could do cross-subslice thread barriers already on SKL but
   not cross-subslice SLM), it may change in the future though.  Even if
   workgroup threads could be spread across subslices it may be
   unnecessary to do anything for memoryBarrierShared(), I don't know
   for sure.

 - The context is not in fault-and-stream mode, which AFAIK we will
   never use on the platforms this affects right now because of hardware
   issues (see "Fault and Stream Gen Graphics » BSpec » Memory Views »
   Faulting [HSW,BDW,SKL+] » Page Faults [SKL+] » Page Fault Modes
   [SKL+] » Fault and Stream") -- Although most likely we will on future
   gens when support for SVM is implemented.

In short, I think it's okay to leave this as a no-op but please add an
XXX comment explaining why it's not necessary to do anything so we
remember to revisit it when any of these three conditions changes.

>
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Francisco Jerez <currojerez at riseup.net>
> ---
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 7eeff93..044b83e 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -1324,6 +1324,9 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
>        break;
>     }
>  
> +   case nir_intrinsic_memory_barrier_atomic_counter:
> +   case nir_intrinsic_memory_barrier_buffer:
> +   case nir_intrinsic_memory_barrier_image:
>     case nir_intrinsic_memory_barrier: {
>        const fs_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_UD, 16 / dispatch_width);
>        bld.emit(SHADER_OPCODE_MEMORY_FENCE, tmp)
> @@ -1331,6 +1334,10 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
>        break;
>     }
>  
> +   case nir_intrinsic_group_memory_barrier:
> +   case nir_intrinsic_memory_barrier_shared:
> +      break;
> +
>     case nir_intrinsic_shader_clock: {
>        /* We cannot do anything if there is an event, so ignore it for now */
>        fs_reg shader_clock = get_timestamp(bld);
> -- 
> 2.6.2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151105/c4b43ed6/attachment.sig>