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.
## 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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
