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, >> + 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
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
