[Mesa-dev] Representing explicit memory layouts in NIR

[Jason Ekstrand]

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.

## 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/8376680f/attachment.html>