[Mesa-dev] [PATCH v3 3/5] nir: convert the glsl intrinsic image_size to nir_intrinsic_image_size

[Francisco Jerez]

Martin Peres <martin.peres at linux.intel.com> writes:

> v2, review from Francisco Jerez:
>  - make the destination variable as large as what the nir instrinsic
>    defines (4) instead of the size of the return variable of glsl. This
>    is still safe for the already existing code because all the intrinsics
>    affected returned the same amount of components as expected by glsl IR.
>    In the case of image_size, it is not possible to do so because the
>    returned number of component depends on the image type and this case
>    is not well handled by nir.
>
> Signed-off-by: Martin Peres <martin.peres at linux.intel.com>
> ---
>  src/glsl/nir/glsl_to_nir.cpp  | 21 +++++++++++++++------
>  src/glsl/nir/nir_intrinsics.h |  2 ++
>  2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index 77327b6..3063243 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -641,6 +641,8 @@ nir_visitor::visit(ir_call *ir)
>           op = nir_intrinsic_image_atomic_comp_swap;
>        } else if (strcmp(ir->callee_name(), "__intrinsic_memory_barrier") == 0) {
>           op = nir_intrinsic_memory_barrier;
> +      } else if (strcmp(ir->callee_name(), "__intrinsic_image_size") == 0) {
> +         op = nir_intrinsic_image_size;
>        } else {
>           unreachable("not reached");
>        }
> @@ -666,7 +668,8 @@ nir_visitor::visit(ir_call *ir)
>        case nir_intrinsic_image_atomic_or:
>        case nir_intrinsic_image_atomic_xor:
>        case nir_intrinsic_image_atomic_exchange:
> -      case nir_intrinsic_image_atomic_comp_swap: {
> +      case nir_intrinsic_image_atomic_comp_swap:
> +      case nir_intrinsic_image_size: {
>           nir_ssa_undef_instr *instr_undef =
>              nir_ssa_undef_instr_create(shader, 1);
>           nir_instr_insert_after_cf_list(this->cf_node_list,
> @@ -681,6 +684,17 @@ nir_visitor::visit(ir_call *ir)
>           instr->variables[0] = evaluate_deref(&instr->instr, image);
>           param = param->get_next();
>  
> +         /* Set the intrinsic destination. */
> +         if (ir->return_deref) {
> +            const nir_intrinsic_info *info;
> +            info = &nir_intrinsic_infos[instr->intrinsic];

I'm a fan of initializing things at the same point where they are
defined.  You only use it once though, you may want to drop the
declaration altogether.

> +            nir_ssa_dest_init(&instr->instr, &instr->dest,
> +                              info->dest_components, NULL);
> +         }
> +
> +         if (op == nir_intrinsic_image_size)
> +            break;
> +
>           /* Set the address argument, extending the coordinate vector to four
>            * components.
>            */
> @@ -721,11 +735,6 @@ nir_visitor::visit(ir_call *ir)
>              instr->src[3] = evaluate_rvalue((ir_dereference *)param);
>              param = param->get_next();
>           }
> -
> -         /* Set the intrinsic destination. */
> -         if (ir->return_deref)
> -            nir_ssa_dest_init(&instr->instr, &instr->dest,
> -                              ir->return_deref->type->vector_elements, NULL);
>           break;
>        }
>        case nir_intrinsic_memory_barrier:
> diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
> index bc6e6b8..6c7a61a 100644
> --- a/src/glsl/nir/nir_intrinsics.h
> +++ b/src/glsl/nir/nir_intrinsics.h
> @@ -123,6 +123,8 @@ INTRINSIC(image_atomic_or, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
>  INTRINSIC(image_atomic_xor, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
>  INTRINSIC(image_atomic_exchange, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
>  INTRINSIC(image_atomic_comp_swap, 4, ARR(4, 1, 1, 1), true, 1, 1, 0, 0)
> +INTRINSIC(image_size, 0, ARR(), true, 4, 1, 0,
> +          NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>  

Looks good to me,
Reviewed-by: Francisco Jerez <currojerez at riseup.net>

>  #define SYSTEM_VALUE(name, components) \
>     INTRINSIC(load_##name, 0, ARR(), true, components, 0, 0, \
> -- 
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- 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/20150819/1d285658/attachment.sig>