[Mesa-dev] [PATCH 1/5] nir: Define image load, store and atomic intrinsics.

[Connor Abbott]

On IRC, Ken and I were discussing using a scheme inspired by SPIR-V,
which has an OpImagePointer instruction that forms a pointer to the
particular texel of the image as well as
OpAtomic{Load,Store,Exchange,etc.} that operate on an image or shared
buffer pointer. The advantages would be:

* Makes translating from SPIR-V easier.
* Reduces the number of intrinsics we need to add for SSBO support.
* Reduces the combinatorial explosion enough that we can have separate
versions for 2, 3, and 4 components and MS vs. non-MS without it being
unbearable. I'm not sure how much of a benefit that would be though.

The disadvantages I can think of are:

* Doesn't actually save any code in the i965 backend, since we need to
do different things depending on if the pointer is to an image or a
shared buffer anyways.
* We'd have to special case nir_convert_from_ssa to ignore the SSA
value that's really a pointer since we don't have any real type-level
support for pointers.
* Since we lower to SSA before converting to i965, there are some ugly
edge cases when the coordinate argument becomes part of a phi web and
gets potentially overwritten before the instruction that uses the
pointer.

I don't have a preference one way or the other, and I guess we could
always refactor it later if we wanted to, so assuming Ken is OK with
this, then besides one minor comment on patch 4 the series is

Reviewed-by: Connor Abbott <cwabbott0 at gmail.com>

On Tue, May 5, 2015 at 4:29 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> ---
>  src/glsl/nir/nir_intrinsics.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
> index 8e28765..4b13c75 100644
> --- a/src/glsl/nir/nir_intrinsics.h
> +++ b/src/glsl/nir/nir_intrinsics.h
> @@ -89,6 +89,33 @@ ATOMIC(inc, 0)
>  ATOMIC(dec, 0)
>  ATOMIC(read, NIR_INTRINSIC_CAN_ELIMINATE)
>
> +/*
> + * Image load, store and atomic intrinsics.
> + *
> + * All image intrinsics take an image target passed as a nir_variable.  Image
> + * variables contain a number of memory and layout qualifiers that influence
> + * the semantics of the intrinsic.
> + *
> + * All image intrinsics take a four-coordinate vector and a sample index as
> + * first two sources, determining the location within the image that will be
> + * accessed by the intrinsic.  Components not applicable to the image target
> + * in use are equal to zero by convention.  Image store takes an additional
> + * four-component argument with the value to be written, and image atomic
> + * operations take either one or two additional scalar arguments with the same
> + * meaning as in the ARB_shader_image_load_store specification.
> + */
> +INTRINSIC(image_load, 2, ARR(4, 1), true, 4, 1, 0,
> +          NIR_INTRINSIC_CAN_ELIMINATE)
> +INTRINSIC(image_store, 3, ARR(4, 1, 4), false, 0, 1, 0, 0)
> +INTRINSIC(image_atomic_add, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
> +INTRINSIC(image_atomic_min, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
> +INTRINSIC(image_atomic_max, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
> +INTRINSIC(image_atomic_and, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
> +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)
> +
>  #define SYSTEM_VALUE(name, components) \
>     INTRINSIC(load_##name, 0, ARR(), true, components, 0, 0, \
>     NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
> --
> 2.3.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev