[Mesa-dev] [PATCH v2] nir: split SSBO min/max atomic instrinsics into signed/unsigned versions

Iago Toral itoral at igalia.com
Mon Oct 5 23:07:33 PDT 2015


Hi,

On Tue, 2015-09-29 at 09:25 +0200, Iago Toral Quiroga wrote:
> NIR is typeless so this is the only way to keep track of the
> type to select the proper atomic to use.
> 
> v2:
>   - Use imin,imax,umin,umax for the intrinsic names (Connor Abbott)
>   - Change message for unreachable paths (Michael Schellenberger)

Since this changes the intrinsics we define for these things I'd like to
push this sooner rather than later. There have been no comments to v2 so
far, any of you feel like giving the Rb to this?

Thanks
Iago

> ---
>  src/glsl/nir/glsl_to_nir.cpp               | 22 ++++++++++++++++++----
>  src/glsl/nir/nir_intrinsics.h              |  6 ++++--
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp   | 20 ++++++++++----------
>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 22 +++++++++++-----------
>  4 files changed, 43 insertions(+), 27 deletions(-)
> 
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index f03a107..25f41a7 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -662,9 +662,21 @@ nir_visitor::visit(ir_call *ir)
>        } else if (strcmp(ir->callee_name(), "__intrinsic_ssbo_atomic_xor_internal") == 0) {
>           op = nir_intrinsic_ssbo_atomic_xor;
>        } else if (strcmp(ir->callee_name(), "__intrinsic_ssbo_atomic_min_internal") == 0) {
> -         op = nir_intrinsic_ssbo_atomic_min;
> +         assert(ir->return_deref);
> +         if (ir->return_deref->type == glsl_type::int_type)
> +            op = nir_intrinsic_ssbo_atomic_imin;
> +         else if (ir->return_deref->type == glsl_type::uint_type)
> +            op = nir_intrinsic_ssbo_atomic_umin;
> +         else
> +            unreachable("Invalid type");
>        } else if (strcmp(ir->callee_name(), "__intrinsic_ssbo_atomic_max_internal") == 0) {
> -         op = nir_intrinsic_ssbo_atomic_max;
> +         assert(ir->return_deref);
> +         if (ir->return_deref->type == glsl_type::int_type)
> +            op = nir_intrinsic_ssbo_atomic_imax;
> +         else if (ir->return_deref->type == glsl_type::uint_type)
> +            op = nir_intrinsic_ssbo_atomic_umax;
> +         else
> +            unreachable("Invalid type");
>        } else if (strcmp(ir->callee_name(), "__intrinsic_ssbo_atomic_exchange_internal") == 0) {
>           op = nir_intrinsic_ssbo_atomic_exchange;
>        } else if (strcmp(ir->callee_name(), "__intrinsic_ssbo_atomic_comp_swap_internal") == 0) {
> @@ -874,8 +886,10 @@ nir_visitor::visit(ir_call *ir)
>           break;
>        }
>        case nir_intrinsic_ssbo_atomic_add:
> -      case nir_intrinsic_ssbo_atomic_min:
> -      case nir_intrinsic_ssbo_atomic_max:
> +      case nir_intrinsic_ssbo_atomic_imin:
> +      case nir_intrinsic_ssbo_atomic_umin:
> +      case nir_intrinsic_ssbo_atomic_imax:
> +      case nir_intrinsic_ssbo_atomic_umax:
>        case nir_intrinsic_ssbo_atomic_and:
>        case nir_intrinsic_ssbo_atomic_or:
>        case nir_intrinsic_ssbo_atomic_xor:
> diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
> index 06f1b02..ff42bb2 100644
> --- a/src/glsl/nir/nir_intrinsics.h
> +++ b/src/glsl/nir/nir_intrinsics.h
> @@ -174,8 +174,10 @@ INTRINSIC(image_samples, 0, ARR(), true, 1, 1, 0,
>   * 3: For CompSwap only: the second data parameter.
>   */
>  INTRINSIC(ssbo_atomic_add, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> -INTRINSIC(ssbo_atomic_min, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> -INTRINSIC(ssbo_atomic_max, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> +INTRINSIC(ssbo_atomic_imin, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> +INTRINSIC(ssbo_atomic_umin, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> +INTRINSIC(ssbo_atomic_imax, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> +INTRINSIC(ssbo_atomic_umax, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
>  INTRINSIC(ssbo_atomic_and, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
>  INTRINSIC(ssbo_atomic_or, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
>  INTRINSIC(ssbo_atomic_xor, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index a2bc5c6..0b9555c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -1870,17 +1870,17 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
>     case nir_intrinsic_ssbo_atomic_add:
>        nir_emit_ssbo_atomic(bld, BRW_AOP_ADD, instr);
>        break;
> -   case nir_intrinsic_ssbo_atomic_min:
> -      if (dest.type == BRW_REGISTER_TYPE_D)
> -         nir_emit_ssbo_atomic(bld, BRW_AOP_IMIN, instr);
> -      else
> -         nir_emit_ssbo_atomic(bld, BRW_AOP_UMIN, instr);
> +   case nir_intrinsic_ssbo_atomic_imin:
> +      nir_emit_ssbo_atomic(bld, BRW_AOP_IMIN, instr);
>        break;
> -   case nir_intrinsic_ssbo_atomic_max:
> -      if (dest.type == BRW_REGISTER_TYPE_D)
> -         nir_emit_ssbo_atomic(bld, BRW_AOP_IMAX, instr);
> -      else
> -         nir_emit_ssbo_atomic(bld, BRW_AOP_UMAX, instr);
> +   case nir_intrinsic_ssbo_atomic_umin:
> +      nir_emit_ssbo_atomic(bld, BRW_AOP_UMIN, instr);
> +      break;
> +   case nir_intrinsic_ssbo_atomic_imax:
> +      nir_emit_ssbo_atomic(bld, BRW_AOP_IMAX, instr);
> +      break;
> +   case nir_intrinsic_ssbo_atomic_umax:
> +      nir_emit_ssbo_atomic(bld, BRW_AOP_UMAX, instr);
>        break;
>     case nir_intrinsic_ssbo_atomic_and:
>        nir_emit_ssbo_atomic(bld, BRW_AOP_AND, instr);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> index 2d2e575..0522fd6 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> @@ -769,17 +769,17 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
>     case nir_intrinsic_ssbo_atomic_add:
>        nir_emit_ssbo_atomic(BRW_AOP_ADD, instr);
>        break;
> -   case nir_intrinsic_ssbo_atomic_min:
> -      if (dest.type == BRW_REGISTER_TYPE_D)
> -         nir_emit_ssbo_atomic(BRW_AOP_IMIN, instr);
> -      else
> -         nir_emit_ssbo_atomic(BRW_AOP_UMIN, instr);
> -      break;
> -   case nir_intrinsic_ssbo_atomic_max:
> -      if (dest.type == BRW_REGISTER_TYPE_D)
> -         nir_emit_ssbo_atomic(BRW_AOP_IMAX, instr);
> -      else
> -         nir_emit_ssbo_atomic(BRW_AOP_UMAX, instr);
> +   case nir_intrinsic_ssbo_atomic_imin:
> +      nir_emit_ssbo_atomic(BRW_AOP_IMIN, instr);
> +      break;
> +   case nir_intrinsic_ssbo_atomic_umin:
> +      nir_emit_ssbo_atomic(BRW_AOP_UMIN, instr);
> +      break;
> +   case nir_intrinsic_ssbo_atomic_imax:
> +      nir_emit_ssbo_atomic(BRW_AOP_IMAX, instr);
> +      break;
> +   case nir_intrinsic_ssbo_atomic_umax:
> +      nir_emit_ssbo_atomic(BRW_AOP_UMAX, instr);
>        break;
>     case nir_intrinsic_ssbo_atomic_and:
>        nir_emit_ssbo_atomic(BRW_AOP_AND, instr);




More information about the mesa-dev mailing list