[Mesa-dev] [PATCH 3/3] nir/spirv: Use fill_common_atomic_sources for image atomics

[Michael Schellenberger Costa]

Hi Jason,

Am 07.09.2016 um 00:17 schrieb Jason Ekstrand:
> We had two almost identical copies of this code and they were both broken
> but in different ways.  The previous two commits fixed both of them.  This
> one just unifies them so that it's easier to handle in the future.
> 
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> ---
>  src/compiler/spirv/spirv_to_nir.c | 99 +++++++++++++++++----------------------
>  1 file changed, 43 insertions(+), 56 deletions(-)
> 
> diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c
> index 0d6a70e..e91a7b2 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -1589,6 +1589,47 @@ vtn_handle_texture(struct vtn_builder *b, SpvOp opcode,
>     nir_builder_instr_insert(&b->nb, &instr->instr);
>  }
>  
> +static void
> +fill_common_atomic_sources(struct vtn_builder *b, SpvOp opcode,
> +                           const uint32_t *w, nir_src *src)
> +{
> +   switch (opcode) {
> +   case SpvOpAtomicIIncrement:
> +      src[0] = nir_src_for_ssa(nir_imm_int(&b->nb, 1));
> +      break;
> +
> +   case SpvOpAtomicIDecrement:
> +      src[0] = nir_src_for_ssa(nir_imm_int(&b->nb, -1));
> +      break;
> +
> +   case SpvOpAtomicISub:
> +      src[0] =
> +         nir_src_for_ssa(nir_ineg(&b->nb, vtn_ssa_value(b, w[6])->def));
> +      break;
> +
> +   case SpvOpAtomicCompareExchange:
> +      src[0] = nir_src_for_ssa(vtn_ssa_value(b, w[8])->def);
> +      src[1] = nir_src_for_ssa(vtn_ssa_value(b, w[7])->def);
> +      break;
> +      /* Fall through */
That fall through seems wrong.
--Michael
> +
> +   case SpvOpAtomicExchange:
> +   case SpvOpAtomicIAdd:
> +   case SpvOpAtomicSMin:
> +   case SpvOpAtomicUMin:
> +   case SpvOpAtomicSMax:
> +   case SpvOpAtomicUMax:
> +   case SpvOpAtomicAnd:
> +   case SpvOpAtomicOr:
> +   case SpvOpAtomicXor:
> +      src[0] = nir_src_for_ssa(vtn_ssa_value(b, w[6])->def);
> +      break;
> +
> +   default:
> +      unreachable("Invalid SPIR-V atomic");
> +   }
> +}
> +
>  static nir_ssa_def *
>  get_image_coord(struct vtn_builder *b, uint32_t value)
>  {
> @@ -1729,13 +1770,9 @@ vtn_handle_image(struct vtn_builder *b, SpvOp opcode,
>     case SpvOpImageWrite:
>        intrin->src[2] = nir_src_for_ssa(vtn_ssa_value(b, w[3])->def);
>        break;
> +
>     case SpvOpAtomicIIncrement:
> -      intrin->src[2] = nir_src_for_ssa(nir_imm_int(&b->nb, 1));
> -      break;
>     case SpvOpAtomicIDecrement:
> -      intrin->src[2] = nir_src_for_ssa(nir_imm_int(&b->nb, -1));
> -      break;
> -
>     case SpvOpAtomicExchange:
>     case SpvOpAtomicIAdd:
>     case SpvOpAtomicSMin:
> @@ -1745,16 +1782,7 @@ vtn_handle_image(struct vtn_builder *b, SpvOp opcode,
>     case SpvOpAtomicAnd:
>     case SpvOpAtomicOr:
>     case SpvOpAtomicXor:
> -      intrin->src[2] = nir_src_for_ssa(vtn_ssa_value(b, w[6])->def);
> -      break;
> -
> -   case SpvOpAtomicCompareExchange:
> -      intrin->src[2] = nir_src_for_ssa(vtn_ssa_value(b, w[8])->def);
> -      intrin->src[3] = nir_src_for_ssa(vtn_ssa_value(b, w[7])->def);
> -      break;
> -
> -   case SpvOpAtomicISub:
> -      intrin->src[2] = nir_src_for_ssa(nir_ineg(&b->nb, vtn_ssa_value(b, w[6])->def));
> +      fill_common_atomic_sources(b, opcode, w, &intrin->src[2]);
>        break;
>  
>     default:
> @@ -1829,47 +1857,6 @@ get_shared_nir_atomic_op(SpvOp opcode)
>  }
>  
>  static void
> -fill_common_atomic_sources(struct vtn_builder *b, SpvOp opcode,
> -                           const uint32_t *w, nir_src *src)
> -{
> -   switch (opcode) {
> -   case SpvOpAtomicIIncrement:
> -      src[0] = nir_src_for_ssa(nir_imm_int(&b->nb, 1));
> -      break;
> -
> -   case SpvOpAtomicIDecrement:
> -      src[0] = nir_src_for_ssa(nir_imm_int(&b->nb, -1));
> -      break;
> -
> -   case SpvOpAtomicISub:
> -      src[0] =
> -         nir_src_for_ssa(nir_ineg(&b->nb, vtn_ssa_value(b, w[6])->def));
> -      break;
> -
> -   case SpvOpAtomicCompareExchange:
> -      src[0] = nir_src_for_ssa(vtn_ssa_value(b, w[8])->def);
> -      src[1] = nir_src_for_ssa(vtn_ssa_value(b, w[7])->def);
> -      break;
> -      /* Fall through */
> -
> -   case SpvOpAtomicExchange:
> -   case SpvOpAtomicIAdd:
> -   case SpvOpAtomicSMin:
> -   case SpvOpAtomicUMin:
> -   case SpvOpAtomicSMax:
> -   case SpvOpAtomicUMax:
> -   case SpvOpAtomicAnd:
> -   case SpvOpAtomicOr:
> -   case SpvOpAtomicXor:
> -      src[0] = nir_src_for_ssa(vtn_ssa_value(b, w[6])->def);
> -      break;
> -
> -   default:
> -      unreachable("Invalid SPIR-V atomic");
> -   }
> -}
> -
> -static void
>  vtn_handle_ssbo_or_shared_atomic(struct vtn_builder *b, SpvOp opcode,
>                                   const uint32_t *w, unsigned count)
>  {
>