Re: [Mesa-dev] [PATCH 1/5] nir: Define image load, store and atomic intrinsics.
Connor Abbott cwabbo...@gmail.com writes:
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.
Yeah, I actually played around with a SPIR-V-like approach, and decided
to give up the idea in the end mainly because of the disadvantages you
mention, because it's nothing close to what the back-ends will want.
Two other ideas occurred to me that could have made the back-end's life
easier, but it didn't seem like they were worth doing:
- Write a driver-independent lowering pass to convert SPIR-V-style
intrinsics into coordinate-based intrinsics while still in SSA form.
In that case there's likely no point in having the SPIR-V-style
intrinsics in the first place, no back-end I know of will prefer
image intrinsics in that form at this point, and we can just let the
SPIR-V front-end deal with this problem alone.
- Actually agree on a representation for a texel pointer. A texel
pointer would be a triple of pointer-to-object, vec4 of coordinates
and sample index, together with some static metadata determining the
memory access properties. Something more back-end-specific would
likely work too. In any case we would also have to agree on a
representation for a pointer to an image/SSB object. And we would
need to type-check pointers correctly to prevent the optimizer from
doing illegal transformations (e.g. a single store intrinsic that
writes to either coherent or non-coherent memory depending on the
parent block, or, if we adhere to the limitations of GLSL strictly, a
single store intrinsic that might access images based on different
array uniforms).
It gets even more interesting if you consider the limitations some
back-ends have about accessing images non-uniformly -- We would have
to guarantee that the pointer-to-object inside the texel pointer has
the same value for all thread invocations executing a given load,
store or atomic intrinsic, what implies that we would have to forbid
texel pointers in phi instructions unless it can be proven that
either the control flow incident into the node is uniform *or* the
pointer-to-object coming from all parent branches is the same.
Sounds like a lot of work for little benefit at this point.
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 cwabbo...@gmail.com
Thanks.
On Tue, May 5, 2015 at 4:29 PM, Francisco Jerez curroje...@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,
+
Re: [Mesa-dev] [PATCH 1/5] nir: Define image load, store and atomic intrinsics.
On Tuesday, May 05, 2015 11:29:52 PM Francisco Jerez 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) Looks great, Curro. Expanding the coordinate to a vec4 and always having the parameters present for e.g. sample_index does reduce the combinatorial explosion a lot. FWIW, I also prefer undefined. This should work out fine for SPIR-V too, it's pretty trivial to go from their style to this (just combine the two - it's SSA after all). These 5 are: Reviewed-by: Kenneth Graunke kenn...@whitecape.org signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] nir: Define image load, store and atomic intrinsics.
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 cwabbo...@gmail.com
On Tue, May 5, 2015 at 4:29 PM, Francisco Jerez curroje...@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/5] nir: Define image load, store and atomic intrinsics.
--- 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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
