[Mesa-dev] [PATCH] nir: split SSBO min/max atomic instrinsics into signed/unsigned versions
Kristian Høgsberg
krh at bitplanet.net
Mon Sep 28 13:55:26 PDT 2015
On Mon, Sep 28, 2015 at 1:47 AM, Iago Toral Quiroga <itoral at igalia.com> wrote:
> NIR is typeless so this is the only way to keep track of the
> type to select the proper atomic to use.
> ---
> I decided to squash the i965 changes in because otherwise we would break
> the build between the nir and i965 patches. Let me know if we rather
> split them anyway.
>
> 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..27e9276 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_min_d;
> + else if (ir->return_deref->type == glsl_type::uint_type)
> + op = nir_intrinsic_ssbo_atomic_min_ud;
> + else
> + unreachable("Not reached");
> } 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_max_d;
> + else if (ir->return_deref->type == glsl_type::uint_type)
> + op = nir_intrinsic_ssbo_atomic_max_ud;
> + else
> + unreachable("Not reached");
> } 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_min_d:
> + case nir_intrinsic_ssbo_atomic_min_ud:
> + case nir_intrinsic_ssbo_atomic_max_d:
> + case nir_intrinsic_ssbo_atomic_max_ud:
> 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..956e68d 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_min_d, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> +INTRINSIC(ssbo_atomic_min_ud, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> +INTRINSIC(ssbo_atomic_max_d, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
> +INTRINSIC(ssbo_atomic_max_ud, 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..9fcddab 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_min_d:
> + 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_min_ud:
> + nir_emit_ssbo_atomic(bld, BRW_AOP_UMIN, instr);
> + break;
> + case nir_intrinsic_ssbo_atomic_max_d:
> + nir_emit_ssbo_atomic(bld, BRW_AOP_IMAX, instr);
> + break;
> + case nir_intrinsic_ssbo_atomic_max_ud:
> + nir_emit_ssbo_atomic(bld, BRW_AOP_UMAX, instr);
> break;
Francisco did something else for image atomics:
/* Get the referenced image variable and type. */
const nir_variable *var = instr->variables[0]->var;
const glsl_type *type = var->type->without_array();
and then determine the atomic operand based on the intrinsic and the
type, like you did in your original approach. Just this time, the type
is actually valid. I think that's a nicer approach and we should
definitely do it the same way for images and SSBOs.
Kristian
> 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..30fa685 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_min_d:
> + nir_emit_ssbo_atomic(BRW_AOP_IMIN, instr);
> + break;
> + case nir_intrinsic_ssbo_atomic_min_ud:
> + nir_emit_ssbo_atomic(BRW_AOP_UMIN, instr);
> + break;
> + case nir_intrinsic_ssbo_atomic_max_d:
> + nir_emit_ssbo_atomic(BRW_AOP_IMAX, instr);
> + break;
> + case nir_intrinsic_ssbo_atomic_max_ud:
> + nir_emit_ssbo_atomic(BRW_AOP_UMAX, instr);
> break;
> case nir_intrinsic_ssbo_atomic_and:
> nir_emit_ssbo_atomic(BRW_AOP_AND, instr);
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list