[Mesa-dev] [PATCH v2 21/42] glsl: Add default matrix ordering in lower_buffer_access

[Jordan Justen]

On 2015-11-25 01:32:37, Iago Toral wrote:
> 
> On Tue, 2015-11-17 at 21:54 -0800, Jordan Justen wrote:
> > For compute shader shared variable we will set a default of column
> > major.
> > 
> > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > ---
> >  src/glsl/lower_buffer_access.cpp |  5 +++--
> >  src/glsl/lower_buffer_access.h   | 10 ++++++++++
> >  2 files changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/glsl/lower_buffer_access.cpp b/src/glsl/lower_buffer_access.cpp
> > index 297ed69..66e7abe 100644
> > --- a/src/glsl/lower_buffer_access.cpp
> > +++ b/src/glsl/lower_buffer_access.cpp
> > @@ -281,8 +281,9 @@ lower_buffer_access::is_dereferenced_thing_row_major(const ir_rvalue *deref)
> >  
> >           switch (matrix_layout) {
> >           case GLSL_MATRIX_LAYOUT_INHERITED:
> > -            assert(!matrix);
> > -            return false;
> > +            assert(default_matrix_layout != GLSL_MATRIX_LAYOUT_INHERITED ||
> > +                   !matrix);
> > +            return default_matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
> 
> I am not sure I understand this. If shared variables are column major by
> default, then isn't that the same behavior we have for ubos and ssbos?
> In what case is this needed?

I think some code must walk the interface blocks an assign the
interface matrix layouts to matrices which are set to inherit their
layout.

Since shared variables are not part of an interface block, this
doesn't happen for matrices in shared variables. This leads to us
hitting the previous assert(!matrix) code.

Instead of this change we could also go through and mark all matrices
in shared variables with a layout other than 'inherited' at an earlier
stage.

-Jordan

> 
> >           case GLSL_MATRIX_LAYOUT_COLUMN_MAJOR:
> >              return false;
> >           case GLSL_MATRIX_LAYOUT_ROW_MAJOR:
> > diff --git a/src/glsl/lower_buffer_access.h b/src/glsl/lower_buffer_access.h
> > index f8e1070..82b35ed 100644
> > --- a/src/glsl/lower_buffer_access.h
> > +++ b/src/glsl/lower_buffer_access.h
> > @@ -39,6 +39,14 @@ namespace lower_buffer_access {
> >  
> >  class lower_buffer_access : public ir_rvalue_enter_visitor {
> >  public:
> > +   lower_buffer_access() :
> > +      default_matrix_layout(GLSL_MATRIX_LAYOUT_INHERITED)
> > +   {}
> > +
> > +   lower_buffer_access(enum glsl_matrix_layout default_matrix_layout) :
> > +      default_matrix_layout(default_matrix_layout)
> > +   {}
> > +
> >     virtual void
> >     insert_buffer_access(void *mem_ctx, ir_dereference *deref,
> >                          const glsl_type *type, ir_rvalue *offset,
> > @@ -55,6 +63,8 @@ public:
> >                              ir_rvalue **offset, unsigned *const_offset,
> >                              bool *row_major, int *matrix_columns,
> >                              unsigned packing);
> > +
> > +   enum glsl_matrix_layout default_matrix_layout;
> >  };
> >  
> >  } /* namespace lower_buffer_access */
> 
> 
>