[Mesa-dev] [PATCH 13/16] nir: Fix OpAtomicCounterIDecrement for uniform atomic counters

[Timothy Arceri]

Thanks.

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

On 03/07/18 00:58, Alejandro Piñeiro wrote:
> From: Antia Puentes <apuentes at igalia.com>
> 
>  From the SPIR-V 1.0 specification, section 3.32.18, "Atomic
> Instructions":
> 
>     "OpAtomicIDecrement:
>      <skip>
>      The instruction's result is the Original Value."
> 
> However, we were implementing it, for uniform atomic counters, as a
> pre-decrement operation, as was the one available from GLSL.
> 
> Renamed the former nir intrinsic 'atomic_counter_dec*' to
> 'atomic_counter_pre_dec*' for clarification purposes, as it implements
> a pre-decrement operation as specified for GLSL. From GLSL 4.50 spec,
> section 8.10, "Atomic Counter Functions":
> 
>     "uint atomicCounterDecrement (atomic_uint c)
> 
>      Atomically
>      1. decrements the counter for c, and
>      2. returns the value resulting from the decrement operation.
> 
>      These two steps are done atomically with respect to the atomic
>      counter functions in this table."
> 
> Added a new nir intrinsic 'atomic_counter_post_dec*' which implements
> a post-decrement operation as required by SPIR-V.
> 
> v2: (Timothy Arceri)
>     * Add extra spec quotes on commit message
>     * Use "post" instead "pos" to avoid confusion with "position"
> 
> Signed-off-by: Antia Puentes <apuentes at igalia.com>
> Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com>
> ---
>   src/compiler/glsl/gl_nir_lower_atomics.c     | 8 ++++++--
>   src/compiler/glsl/glsl_to_nir.cpp            | 4 ++--
>   src/compiler/nir/nir_intrinsics.py           | 3 ++-
>   src/compiler/nir/nir_lower_atomics_to_ssbo.c | 8 +++++---
>   src/compiler/spirv/spirv_to_nir.c            | 2 +-
>   5 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/src/compiler/glsl/gl_nir_lower_atomics.c b/src/compiler/glsl/gl_nir_lower_atomics.c
> index 293730966fd..36e273c45d2 100644
> --- a/src/compiler/glsl/gl_nir_lower_atomics.c
> +++ b/src/compiler/glsl/gl_nir_lower_atomics.c
> @@ -53,8 +53,12 @@ lower_deref_instr(nir_builder *b, nir_intrinsic_instr *instr,
>         op = nir_intrinsic_atomic_counter_inc;
>         break;
>   
> -   case nir_intrinsic_atomic_counter_dec_deref:
> -      op = nir_intrinsic_atomic_counter_dec;
> +   case nir_intrinsic_atomic_counter_pre_dec_deref:
> +      op = nir_intrinsic_atomic_counter_pre_dec;
> +      break;
> +
> +   case nir_intrinsic_atomic_counter_post_dec_deref:
> +      op = nir_intrinsic_atomic_counter_post_dec;
>         break;
>   
>      case nir_intrinsic_atomic_counter_add_deref:
> diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp
> index d3a3fb9b085..2d76c7e6cfe 100644
> --- a/src/compiler/glsl/glsl_to_nir.cpp
> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> @@ -630,7 +630,7 @@ nir_visitor::visit(ir_call *ir)
>            op = nir_intrinsic_atomic_counter_inc_deref;
>            break;
>         case ir_intrinsic_atomic_counter_predecrement:
> -         op = nir_intrinsic_atomic_counter_dec_deref;
> +         op = nir_intrinsic_atomic_counter_pre_dec_deref;
>            break;
>         case ir_intrinsic_atomic_counter_add:
>            op = nir_intrinsic_atomic_counter_add_deref;
> @@ -831,7 +831,7 @@ nir_visitor::visit(ir_call *ir)
>         switch (op) {
>         case nir_intrinsic_atomic_counter_read_deref:
>         case nir_intrinsic_atomic_counter_inc_deref:
> -      case nir_intrinsic_atomic_counter_dec_deref:
> +      case nir_intrinsic_atomic_counter_pre_dec_deref:
>         case nir_intrinsic_atomic_counter_add_deref:
>         case nir_intrinsic_atomic_counter_min_deref:
>         case nir_intrinsic_atomic_counter_max_deref:
> diff --git a/src/compiler/nir/nir_intrinsics.py b/src/compiler/nir/nir_intrinsics.py
> index d9d0bbdfccf..caef6f55d3f 100644
> --- a/src/compiler/nir/nir_intrinsics.py
> +++ b/src/compiler/nir/nir_intrinsics.py
> @@ -270,7 +270,8 @@ def atomic3(name):
>       intrinsic(name, src_comp=[1, 1, 1], dest_comp=1, indices=[BASE])
>   
>   atomic("atomic_counter_inc")
> -atomic("atomic_counter_dec")
> +atomic("atomic_counter_pre_dec")
> +atomic("atomic_counter_post_dec")
>   atomic("atomic_counter_read", flags=[CAN_ELIMINATE])
>   atomic2("atomic_counter_add")
>   atomic2("atomic_counter_min")
> diff --git a/src/compiler/nir/nir_lower_atomics_to_ssbo.c b/src/compiler/nir/nir_lower_atomics_to_ssbo.c
> index 934ae81d750..6ebd3632288 100644
> --- a/src/compiler/nir/nir_lower_atomics_to_ssbo.c
> +++ b/src/compiler/nir/nir_lower_atomics_to_ssbo.c
> @@ -71,7 +71,8 @@ lower_instr(nir_intrinsic_instr *instr, unsigned ssbo_offset, nir_builder *b)
>         return true;
>      case nir_intrinsic_atomic_counter_inc:
>      case nir_intrinsic_atomic_counter_add:
> -   case nir_intrinsic_atomic_counter_dec:
> +   case nir_intrinsic_atomic_counter_pre_dec:
> +   case nir_intrinsic_atomic_counter_post_dec:
>         /* inc and dec get remapped to add: */
>         op = nir_intrinsic_ssbo_atomic_add;
>         break;
> @@ -119,7 +120,8 @@ lower_instr(nir_intrinsic_instr *instr, unsigned ssbo_offset, nir_builder *b)
>         nir_src_copy(&new_instr->src[1], &instr->src[0], new_instr);
>         new_instr->src[2] = nir_src_for_ssa(temp);
>         break;
> -   case nir_intrinsic_atomic_counter_dec:
> +   case nir_intrinsic_atomic_counter_pre_dec:
> +   case nir_intrinsic_atomic_counter_post_dec:
>         /* remapped to ssbo_atomic_add: { buffer_idx, offset, -1 } */
>         /* NOTE semantic difference so we adjust the return value below */
>         temp = nir_imm_int(b, -1);
> @@ -148,7 +150,7 @@ lower_instr(nir_intrinsic_instr *instr, unsigned ssbo_offset, nir_builder *b)
>      nir_instr_insert_before(&instr->instr, &new_instr->instr);
>      nir_instr_remove(&instr->instr);
>   
> -   if (instr->intrinsic == nir_intrinsic_atomic_counter_dec) {
> +   if (instr->intrinsic == nir_intrinsic_atomic_counter_pre_dec) {
>         b->cursor = nir_after_instr(&new_instr->instr);
>         nir_ssa_def *result = nir_iadd(b, &new_instr->dest.ssa, temp);
>         nir_ssa_def_rewrite_uses(&instr->dest.ssa, nir_src_for_ssa(result));
> diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c
> index 9d2f57cef94..fb4211193fb 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -2519,7 +2519,7 @@ get_uniform_nir_atomic_op(struct vtn_builder *b, SpvOp opcode)
>      OP(AtomicExchange,         exchange)
>      OP(AtomicCompareExchange,  comp_swap)
>      OP(AtomicIIncrement,       inc_deref)
> -   OP(AtomicIDecrement,       dec_deref)
> +   OP(AtomicIDecrement,       post_dec_deref)
>      OP(AtomicIAdd,             add_deref)
>      OP(AtomicISub,             add_deref)
>      OP(AtomicUMin,             min_deref)
>