[Mesa-dev] Representing explicit memory layouts in NIR

Fri Nov 30 22:52:22 UTC 2018

On Fri, Nov 30, 2018 at 4:39 PM Timothy Arceri <tarceri at itsqueeze.com>
wrote:

> On 1/12/18 9:11 am, Jason Ekstrand wrote:
> > All,
> >
> > This week, I've been working on trying to move UBO and SSBO access in
> > NIR over to deref instructions.  I'm hoping that this will allow us to
> > start doing alias analysis and copy-propagation on it.  The passes we
> > have in NIR *should* be able to work with SSBOs as long as
> > nir_compare_derefs does the right thing.
> >
> > # A story about derefs
> >
> > In that effort, I've run into a bit of a snag with how to represent the
> > layout information.  What we get in from SPIR-V for Vulkan is a byte
> > offset for every struct member and a byte stride for every array (and
> > pointer in the OpPtrAccessChain case).  For matrices, there is an
> > additional RowMajor boolean we need to track somewhere.  With OpenCL
> > memory access, you don't get these decorations but it's trivial to
> > translate the OpenCL layout (It's the same as C) to offset/stride when
> > creating the type.  I've come up with three different ways to represent
> > the information and they all have their own downsides:
> >
> > ## 1. Put the information on the glsl_type similar to how it's done in
> > SPIR-V
> >
> > This has the advantage of being fairly non-invasive to glsl_type.  A lot
> > of the fields we need are already there and the only real change is to
> > allow array types to have strides.  The downside is that the information
> > is often not where you want.  Arrays and structs are ok but, for
> > matrices, you have to go fishing all the way back to the struct type to
> > get the RowMajor and MatrixStride decorations.  (Thanks, SPIR-V...)
> > While this seems like a local annoyance, it actually destroys basically
> > all the advantages of having the information on the type and makes
> > lower_io a real pain.
> >
> > ## 2. Put the information on the type but do it properly
> >
> > In this version, we would put the matrix stride and RowMajor decoration
> > directly on the matrix type.  One obvious advantage here is that it
> > means no fishing for matrix type information.  Another is that, by
> > having the types specialized like this, the only way to change layouts
> > mid-deref-chain would be to have a cast.  Option 1 doesn't provide this
> > because matrix types are the same regardless of whether or not they're
> > declared RowMajor in the struct.  The downside to this option is that it
> > requires glsl_type surgery to make it work.  More on that in a bit.
> >
> > ## 3. Put the information directly on the deref
> >
> > Instead of putting the stride/offset information on the type, we just
> > put it on the deref as we build the deref chain.  This is easy enough to
> > do in spirv_to_nir and someone could also do it easily enough as a
> > lowering pass based on a type_size function.  This has the advantage of
> > simplicity because you don't have to modify glsl_type at all and
> > lowering is stupid-easy because all the information you need is right
> > there on the deref.  The downside, however, is that you alias analysis
> > is potentially harder because you don't have the nice guarantee that you
> > don't see a layout change without a type cast.  The other downside is
> > that we can't ever use copy_deref with anything bigger than a vector
> > because you don't know the sizes of any types and, unless spirv_to_nir
> > puts the offset/stride information on the deref, there's now way to
> > reconstruct it.
> >
> > I've prototyped both 1 and 3 so far and I definitely like 3 better than
> > 1 but it's not great.  I haven't prototyped 2 yet due to the issue
> > mentioned with glsl_type.
> >
> > Between 2 and 3, I really don't know how much we actually loose in terms
> > of our ability to do alias analysis.  I've written the alias analysis
> > for 3 and it isn't too bad.  I'm also not sure how much we would
> > actually loose from not being able to express whole-array or
> > whole-struct copies.  However, without a good reason otherwise, option 2
> > really seems like it's the best of all worlds....
> >
> > # glsl_type surgery
> >
> > You want a good reason, eh?  You should have known this was coming...
> >
> > The problem with option 2 above is that it requires significant
> > glsl_type surgery to do it.  Putting decorations on matrices violates
> > one of the core principals of glsl_type, namely that all fundamental
> > types: scalars, vectors, matrices, images, and samplers are singletons.
> > Other types such as structs and arrays we build on-the-fly and cache
> > as-needed.  In order to do what we need for option 2 above, you have to
> > at least drop this for matrices and possibly vectors (the columns of a
> > row-major mat4 are vectors with a stride of 16).  Again, I see two
> options:
> >
> > ## A. Major rework of the guts of glsl_type
> >
> > Basically, get rid of the static singletons and just use the build
> > on-the-fly and cache model for everything.  This would mean that mat4 ==
> > mat4 is no longer guaranteed unless you know a priori that none of your
> > types are decorated with layout information.  It would also be, not only
> > a pile of work, but a single mega-patch.  I don't know of any way to
> > make that change without just ripping it all up and putting it back
> > together.
>
> Do we really need to throw away the singleton model? Could we not add
> another type on top of matrices to hold the layout information much like
> how we handle arrays and structs and just strip it off (like we often do
> with arrays) when needed for comparisons?
>
> It's possible this could be messy, just trying to throw so ideas out there.
>

As I've been thinking about that.  My thought for nir_type (if that's a
thing) has been to have a "bare type" pointer that always points back to
the undecorated version to use in comparisons.

--Jason

> >
> > ## B. Make a new nir_type and make NIR use it
> >
> > This seems a bit crazy at this point.  src/compiler/nir itself has over
> > 200 references to glsl_type and that doesn't include back-ends.  It'd be
> > a major overhaul and it's not clear that it's worth it.  However, it
> > would mean that we'd have a chance to rewrite types and maybe do it
> > better.  Basing it on nir_alu_type instead of glsl_base_type would be
> > really nice because nir_alu_type already has an orthogonal split between
> > bit size and format (float, uint, etc.).  I would also likely structure
> > it like vtn_type which has a different base_type concept which I find
> > works better than glsl_base_type.
> >
> > Of course, A would be less invasive than B but B would give us the
> > chance to improve some things without rewriting quite as many levels of
> > the compiler.  There are a number of things I think we could do better
> > but changing those in the GLSL compiler would be a *lot* of work
> > especially since it doesn't use the C helpers that NIR does.  On the
> > other hand, the churn in NIR from introducing a new type data structure
> > would be pretty big.  I did a quick git grep and it looks like most of
> > the back-ends make pretty light use of glsl_type when it consuming NIR
> > so maybe it wouldn't be that bad?
> >
> > Thoughts?  Questions?  Objections?
> >
> > --Jason
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181130/2031f73a/attachment-0001.html>