Re: [Mesa-dev] Representing explicit memory layouts in NIR

2018-12-03 Thread apinheiro

On 1/12/18 14:34, Jason Ekstrand wrote:
> On Sat, Dec 1, 2018 at 4:06 AM apinheiro  > wrote:
>
> On 30/11/18 23:52, Jason Ekstrand wrote:
>> On Fri, Nov 30, 2018 at 4:39 PM Timothy Arceri
>> mailto:tarc...@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.
>>
>
> FWIW, while working on ARB_gl_spirv I also found annoying the need
> to fish for the struct member information in order to get RowMajor
> and MatrixStride information, as that lead to the need to track
> the parent_type while you were processing the current type. As a
> example, this is a extract of the code from _get_type_size (new
> method from ARB_gl_spirv ubo/ssbo support, used to compute the
> ubo/ssbo size):
>
>    /* Matrices must have a matrix stride and either RowMajor or
> ColMajor */
>    if (glsl_type_is_matrix(type)) {
>   unsigned matrix_stride =
>  glsl_get_struct_field_explicit_matrix_stride(parent_type,
> index_in_parent);
>
>   bool row_major =
>  glsl_get_struct_field_matrix_layout(parent_type,
> index_in_parent) ==
>  GLSL_MATRIX_LAYOUT_ROW_MAJOR;
>
>   unsigned length = row_major ? glsl_get_vector_elements(type)
>  : glsl_get_length(type);
>
>   /* We don't really need to compute the type_size of the
> matrix element
>    * type. That should be already included as part of
> matrix_stride
>    */
>   return matrix_stride * length;
>    }
>
>
> So for a given type, sometimes we are using the info from the
> current type, and sometimes we need to go up to the parent (so in
> addition to the parent_type we also need to track the index of the
> current type on its parent).
>
> At some point I was tempted to move matrix stride and row major
>

Re: [Mesa-dev] Representing explicit memory layouts in NIR

2018-12-01 Thread Jason Ekstrand
For what it's worth, I've prototyped both 1 and 3 and I don't like either:

https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/nir-ubo-deref-v0.2
https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/nir-ubo-deref-v0.3

I'm going to try 2B next week.

--Jason


On Fri, Nov 30, 2018 at 4:11 PM 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. 

Re: [Mesa-dev] Representing explicit memory layouts in NIR

2018-12-01 Thread Jason Ekstrand
On Sat, Dec 1, 2018 at 4:06 AM apinheiro  wrote:

> On 30/11/18 23:52, Jason Ekstrand wrote:
>
> On Fri, Nov 30, 2018 at 4:39 PM Timothy Arceri 
> 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.
>>
>
> FWIW, while working on ARB_gl_spirv I also found annoying the need to fish
> for the struct member information in order to get RowMajor and MatrixStride
> information, as that lead to the need to track the parent_type while you
> were processing the current type. As a example, this is a extract of the
> code from _get_type_size (new method from ARB_gl_spirv ubo/ssbo support,
> used to compute the ubo/ssbo size):
>
>/* Matrices must have a matrix stride and either RowMajor or ColMajor */
>if (glsl_type_is_matrix(type)) {
>   unsigned matrix_stride =
>  glsl_get_struct_field_explicit_matrix_stride(parent_type,
> index_in_parent);
>
>   bool row_major =
>  glsl_get_struct_field_matrix_layout(parent_type, index_in_parent)
> ==
>  GLSL_MATRIX_LAYOUT_ROW_MAJOR;
>
>   unsigned length = row_major ? glsl_get_vector_elements(type)
>  : glsl_get_length(type);
>
>   /* We don't really need to compute the type_size of the matrix
> element
>* type. That should be already included as part of matrix_stride
>*/
>   return matrix_stride * length;
>}
>
>
> So for a given type, sometimes we are using the info from the current
> type, and sometimes we need to go up to the parent (so in addition to the
> parent_type we also need to track the index of the current type on its
> parent).
>
> At some point I was tempted to move matrix stride and row major from the
> struct field to glsl type. But as Jason mentioned, that would need a lot of
> changes, and I felt that doing that only for ARB_gl_spirv convenience was
> an overkill. But if we have now more reasons to do the move, I'm on that
> ship.
>
>
> >
>> > ## 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 

Re: [Mesa-dev] Representing explicit memory layouts in NIR

2018-12-01 Thread apinheiro
On 30/11/18 23:52, Jason Ekstrand wrote:
> On Fri, Nov 30, 2018 at 4:39 PM Timothy Arceri  > 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.
>

FWIW, while working on ARB_gl_spirv I also found annoying the need to
fish for the struct member information in order to get RowMajor and
MatrixStride information, as that lead to the need to track the
parent_type while you were processing the current type. As a example,
this is a extract of the code from _get_type_size (new method from
ARB_gl_spirv ubo/ssbo support, used to compute the ubo/ssbo size):

   /* Matrices must have a matrix stride and either RowMajor or ColMajor */
   if (glsl_type_is_matrix(type)) {
  unsigned matrix_stride =
 glsl_get_struct_field_explicit_matrix_stride(parent_type,
index_in_parent);

  bool row_major =
 glsl_get_struct_field_matrix_layout(parent_type,
index_in_parent) ==
 GLSL_MATRIX_LAYOUT_ROW_MAJOR;

  unsigned length = row_major ? glsl_get_vector_elements(type)
 : glsl_get_length(type);

  /* We don't really need to compute the type_size of the matrix element
   * type. That should be already included as part of matrix_stride
   */
  return matrix_stride * length;
   }


So for a given type, sometimes we are using the info from the current
type, and sometimes we need to go up to the parent (so in addition to
the parent_type we also need to track the index of the current type on
its parent).

At some point I was tempted to move matrix stride and row major from the
struct field to glsl type. But as Jason mentioned, that would need a lot
of changes, and I felt that doing that only for ARB_gl_spirv convenience
was an overkill. But if we have now more reasons to do the move, I'm on
that ship.


> >
> > ## 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 

Re: [Mesa-dev] Representing explicit memory layouts in NIR

2018-11-30 Thread Jason Ekstrand
On Fri, Nov 30, 2018 at 4:39 PM Timothy Arceri 
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 

Re: [Mesa-dev] Representing explicit memory layouts in NIR

2018-11-30 Thread Timothy Arceri

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 

[Mesa-dev] Representing explicit memory layouts in NIR

2018-11-30 Thread 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