[Mesa-dev] glsl: Expand matrix flip optimization pass to cover more cases.

Submitted by Iago Toral Quiroga on April 21, 2014, 11:38 a.m.

Details

Message ID 1398080307-2878-1-git-send-email-itoral@igalia.com
State Superseded
Headers show

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga April 21, 2014, 11:38 a.m.
Currently it only considers the cases of gl_ModelViewProjectionMatrix and
gl_TextureMatrix. The same optimization can be done also for
gl_ModelViewMatrix, gl_ProjectionMatrix and the corresponding inverses.
---
 src/glsl/opt_flip_matrices.cpp | 118 +++++++++++++++++++++++++++++++----------
 1 file changed, 91 insertions(+), 27 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/glsl/opt_flip_matrices.cpp b/src/glsl/opt_flip_matrices.cpp
index 9044fd6..bf09b78 100644
--- a/src/glsl/opt_flip_matrices.cpp
+++ b/src/glsl/opt_flip_matrices.cpp
@@ -29,8 +29,9 @@ 
  * On some hardware, this is more efficient.
  *
  * This currently only does the conversion for built-in matrices which
- * already have transposed equivalents.  Namely, gl_ModelViewProjectionMatrix
- * and gl_TextureMatrix.
+ * already have transposed equivalents. Namely, gl_ModelViewMatrix,
+ * gl_ProjectionMatrix, gl_ModelViewProjectionMatrix, gl_TextureMatrix and
+ * their inverses.
  */
 #include "ir.h"
 #include "ir_optimization.h"
@@ -42,18 +43,39 @@  public:
    matrix_flipper(exec_list *instructions)
    {
       progress = false;
+      p_transpose = NULL;
+      p_inv_transpose = NULL;
+      mv_transpose = NULL;
+      mv_inv_transpose = NULL;
       mvp_transpose = NULL;
+      mvp_inv_transpose = NULL;
       texmat_transpose = NULL;
+      texmat_inv_transpose = NULL;
 
       foreach_list(n, instructions) {
          ir_instruction *ir = (ir_instruction *) n;
          ir_variable *var = ir->as_variable();
          if (!var)
             continue;
-         if (strcmp(var->name, "gl_ModelViewProjectionMatrixTranspose") == 0)
+
+         if (strcmp(var->name, "gl_ProjectionMatrixTranspose") == 0)
+            p_transpose = var;
+         else if (strcmp(var->name, "gl_ProjectionMatrixInverseTranspose") == 0)
+            p_inv_transpose = var;
+         else if (strcmp(var->name, "gl_ModelViewMatrixTranspose") == 0)
+            mv_transpose = var;
+         else if (strcmp(var->name, "gl_ModelViewMatrixInverseTranspose") == 0)
+            mv_inv_transpose = var;
+         else if (strcmp(var->name,
+                         "gl_ModelViewProjectionMatrixTranspose") == 0)
             mvp_transpose = var;
-         if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0)
+         else if (strcmp(var->name,
+                         "gl_ModelViewProjectionMatrixInverseTranspose") == 0)
+            mvp_inv_transpose = var;
+         else if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0)
             texmat_transpose = var;
+         else if (strcmp(var->name, "gl_TextureMatrixInverseTranspose") == 0)
+            texmat_inv_transpose = var;
       }
    }
 
@@ -62,11 +84,49 @@  public:
    bool progress;
 
 private:
+   ir_variable *p_transpose;
+   ir_variable *p_inv_transpose;
+   ir_variable *mv_transpose;
+   ir_variable *mv_inv_transpose;
    ir_variable *mvp_transpose;
+   ir_variable *mvp_inv_transpose;
    ir_variable *texmat_transpose;
+   ir_variable *texmat_inv_transpose;
 };
 }
 
+static void
+transform_operands(ir_expression *ir,
+                   ir_variable *mat_var, ir_variable *mat_transpose)
+{
+#ifndef NDEBUG
+   ir_dereference_variable *deref = ir->operands[0]->as_dereference_variable();
+   assert(deref && deref->var == mat_var);
+#endif
+
+   void *mem_ctx = ralloc_parent(ir);
+   ir->operands[0] = ir->operands[1];
+   ir->operands[1] = new(mem_ctx) ir_dereference_variable(mat_transpose);
+}
+
+static void
+transform_operands_texmat(ir_expression *ir,
+                          ir_variable *mat_var, ir_variable *mat_transpose)
+{
+   ir_dereference_array *array_ref = ir->operands[0]->as_dereference_array();
+   assert(array_ref != NULL);
+   ir_dereference_variable *var_ref = array_ref->array->as_dereference_variable();
+   assert(var_ref && var_ref->var == mat_var);
+
+   ir->operands[0] = ir->operands[1];
+   ir->operands[1] = array_ref;
+
+   var_ref->var = mat_transpose;
+
+   mat_transpose->data.max_array_access =
+      MAX2(mat_transpose->data.max_array_access, mat_var->data.max_array_access);
+}
+
 ir_visitor_status
 matrix_flipper::visit_enter(ir_expression *ir)
 {
@@ -81,32 +141,36 @@  matrix_flipper::visit_enter(ir_expression *ir)
 
    if (mvp_transpose &&
        strcmp(mat_var->name, "gl_ModelViewProjectionMatrix") == 0) {
-#ifndef NDEBUG
-      ir_dereference_variable *deref = ir->operands[0]->as_dereference_variable();
-      assert(deref && deref->var == mat_var);
-#endif
-
-      void *mem_ctx = ralloc_parent(ir);
-
-      ir->operands[0] = ir->operands[1];
-      ir->operands[1] = new(mem_ctx) ir_dereference_variable(mvp_transpose);
-
+      transform_operands(ir, mat_var, mvp_transpose);
+      progress = true;
+   } else if (mvp_inv_transpose &&
+              strcmp(mat_var->name,
+                     "gl_ModelViewProjectionMatrixInverse") == 0) {
+      transform_operands(ir, mat_var, mvp_inv_transpose);
+      progress = true;
+   } else if (p_transpose &&
+              strcmp(mat_var->name, "gl_ProjectionMatrix") == 0) {
+      transform_operands(ir, mat_var, p_transpose);
+      progress = true;
+   } else if (p_inv_transpose &&
+              strcmp(mat_var->name, "gl_ProjectionMatrixInverse") == 0) {
+      transform_operands(ir, mat_var, p_inv_transpose);
+      progress = true;
+   } else if (mv_transpose &&
+              strcmp(mat_var->name, "gl_ModelViewMatrix") == 0) {
+      transform_operands(ir, mat_var, mv_transpose);
+      progress = true;
+   } else if (mv_inv_transpose &&
+              strcmp(mat_var->name, "gl_ModelViewMatrixInverse") == 0) {
+      transform_operands(ir, mat_var, mv_inv_transpose);
       progress = true;
    } else if (texmat_transpose &&
               strcmp(mat_var->name, "gl_TextureMatrix") == 0) {
-      ir_dereference_array *array_ref = ir->operands[0]->as_dereference_array();
-      assert(array_ref != NULL);
-      ir_dereference_variable *var_ref = array_ref->array->as_dereference_variable();
-      assert(var_ref && var_ref->var == mat_var);
-
-      ir->operands[0] = ir->operands[1];
-      ir->operands[1] = array_ref;
-
-      var_ref->var = texmat_transpose;
-
-      texmat_transpose->data.max_array_access =
-         MAX2(texmat_transpose->data.max_array_access, mat_var->data.max_array_access);
-
+      transform_operands_texmat(ir, mat_var, texmat_transpose);
+      progress = true;
+   } else if (texmat_inv_transpose &&
+              strcmp(mat_var->name, "gl_TextureMatrixInverse") == 0) {
+      transform_operands_texmat(ir, mat_var, texmat_inv_transpose);
       progress = true;
    }
 

Comments

On 04/21/2014 04:38 AM, Iago Toral Quiroga wrote:
> Currently it only considers the cases of gl_ModelViewProjectionMatrix and
> gl_TextureMatrix. The same optimization can be done also for
> gl_ModelViewMatrix, gl_ProjectionMatrix and the corresponding inverses.

I've been looking at some thing similar recently.  I've been looking at
flipping user-defined matrices.

I think rather than adding a bunch of individual handles to flippable
matrices, I think we should add a hash-table of possible names.  For the
built-in uniforms, we would add any matrix uniform that doesn't have
"Transpose" in the name.  Something like:

    struct matrix_and_transpose {
        ir_variable *matrix;
        ir_variable *transpose_matrix;
    };

I think all of the names have regular enough patterns that we should be
able to generate the non-transpose name from the transpose name.  Vice
versa shouldn't be necessary.  If the transpose name is encountered
first, add the structure with matrix set NULL and the name derived from
the transpose name.

One other comment below.

> ---
>  src/glsl/opt_flip_matrices.cpp | 118 +++++++++++++++++++++++++++++++----------
>  1 file changed, 91 insertions(+), 27 deletions(-)
> 
> diff --git a/src/glsl/opt_flip_matrices.cpp b/src/glsl/opt_flip_matrices.cpp
> index 9044fd6..bf09b78 100644
> --- a/src/glsl/opt_flip_matrices.cpp
> +++ b/src/glsl/opt_flip_matrices.cpp
> @@ -29,8 +29,9 @@
>   * On some hardware, this is more efficient.
>   *
>   * This currently only does the conversion for built-in matrices which
> - * already have transposed equivalents.  Namely, gl_ModelViewProjectionMatrix
> - * and gl_TextureMatrix.
> + * already have transposed equivalents. Namely, gl_ModelViewMatrix,
> + * gl_ProjectionMatrix, gl_ModelViewProjectionMatrix, gl_TextureMatrix and
> + * their inverses.
>   */
>  #include "ir.h"
>  #include "ir_optimization.h"
> @@ -42,18 +43,39 @@ public:
>     matrix_flipper(exec_list *instructions)
>     {
>        progress = false;
> +      p_transpose = NULL;
> +      p_inv_transpose = NULL;
> +      mv_transpose = NULL;
> +      mv_inv_transpose = NULL;
>        mvp_transpose = NULL;
> +      mvp_inv_transpose = NULL;
>        texmat_transpose = NULL;
> +      texmat_inv_transpose = NULL;
>  
>        foreach_list(n, instructions) {
>           ir_instruction *ir = (ir_instruction *) n;
>           ir_variable *var = ir->as_variable();
>           if (!var)
>              continue;
> -         if (strcmp(var->name, "gl_ModelViewProjectionMatrixTranspose") == 0)
> +
> +         if (strcmp(var->name, "gl_ProjectionMatrixTranspose") == 0)
> +            p_transpose = var;
> +         else if (strcmp(var->name, "gl_ProjectionMatrixInverseTranspose") == 0)
> +            p_inv_transpose = var;
> +         else if (strcmp(var->name, "gl_ModelViewMatrixTranspose") == 0)
> +            mv_transpose = var;
> +         else if (strcmp(var->name, "gl_ModelViewMatrixInverseTranspose") == 0)
> +            mv_inv_transpose = var;
> +         else if (strcmp(var->name,
> +                         "gl_ModelViewProjectionMatrixTranspose") == 0)
>              mvp_transpose = var;
> -         if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0)
> +         else if (strcmp(var->name,
> +                         "gl_ModelViewProjectionMatrixInverseTranspose") == 0)
> +            mvp_inv_transpose = var;
> +         else if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0)
>              texmat_transpose = var;
> +         else if (strcmp(var->name, "gl_TextureMatrixInverseTranspose") == 0)
> +            texmat_inv_transpose = var;
>        }
>     }
>  
> @@ -62,11 +84,49 @@ public:
>     bool progress;
>  
>  private:
> +   ir_variable *p_transpose;
> +   ir_variable *p_inv_transpose;
> +   ir_variable *mv_transpose;
> +   ir_variable *mv_inv_transpose;
>     ir_variable *mvp_transpose;
> +   ir_variable *mvp_inv_transpose;
>     ir_variable *texmat_transpose;
> +   ir_variable *texmat_inv_transpose;
>  };
>  }
>  
> +static void
> +transform_operands(ir_expression *ir,
> +                   ir_variable *mat_var, ir_variable *mat_transpose)
> +{
> +#ifndef NDEBUG
> +   ir_dereference_variable *deref = ir->operands[0]->as_dereference_variable();
> +   assert(deref && deref->var == mat_var);
> +#endif
> +
> +   void *mem_ctx = ralloc_parent(ir);
> +   ir->operands[0] = ir->operands[1];
> +   ir->operands[1] = new(mem_ctx) ir_dereference_variable(mat_transpose);
> +}
> +
> +static void
> +transform_operands_texmat(ir_expression *ir,
> +                          ir_variable *mat_var, ir_variable *mat_transpose)

I'd call this transform_operands_array_of_matrix or something.  Once we
add support for user-defined matrices, this function will be used for
other arrays too.

> +{
> +   ir_dereference_array *array_ref = ir->operands[0]->as_dereference_array();
> +   assert(array_ref != NULL);
> +   ir_dereference_variable *var_ref = array_ref->array->as_dereference_variable();
> +   assert(var_ref && var_ref->var == mat_var);
> +
> +   ir->operands[0] = ir->operands[1];
> +   ir->operands[1] = array_ref;
> +
> +   var_ref->var = mat_transpose;
> +
> +   mat_transpose->data.max_array_access =
> +      MAX2(mat_transpose->data.max_array_access, mat_var->data.max_array_access);
> +}
> +
>  ir_visitor_status
>  matrix_flipper::visit_enter(ir_expression *ir)
>  {
> @@ -81,32 +141,36 @@ matrix_flipper::visit_enter(ir_expression *ir)
>  
>     if (mvp_transpose &&
>         strcmp(mat_var->name, "gl_ModelViewProjectionMatrix") == 0) {
> -#ifndef NDEBUG
> -      ir_dereference_variable *deref = ir->operands[0]->as_dereference_variable();
> -      assert(deref && deref->var == mat_var);
> -#endif
> -
> -      void *mem_ctx = ralloc_parent(ir);
> -
> -      ir->operands[0] = ir->operands[1];
> -      ir->operands[1] = new(mem_ctx) ir_dereference_variable(mvp_transpose);
> -
> +      transform_operands(ir, mat_var, mvp_transpose);
> +      progress = true;
> +   } else if (mvp_inv_transpose &&
> +              strcmp(mat_var->name,
> +                     "gl_ModelViewProjectionMatrixInverse") == 0) {
> +      transform_operands(ir, mat_var, mvp_inv_transpose);
> +      progress = true;
> +   } else if (p_transpose &&
> +              strcmp(mat_var->name, "gl_ProjectionMatrix") == 0) {
> +      transform_operands(ir, mat_var, p_transpose);
> +      progress = true;
> +   } else if (p_inv_transpose &&
> +              strcmp(mat_var->name, "gl_ProjectionMatrixInverse") == 0) {
> +      transform_operands(ir, mat_var, p_inv_transpose);
> +      progress = true;
> +   } else if (mv_transpose &&
> +              strcmp(mat_var->name, "gl_ModelViewMatrix") == 0) {
> +      transform_operands(ir, mat_var, mv_transpose);
> +      progress = true;
> +   } else if (mv_inv_transpose &&
> +              strcmp(mat_var->name, "gl_ModelViewMatrixInverse") == 0) {
> +      transform_operands(ir, mat_var, mv_inv_transpose);
>        progress = true;
>     } else if (texmat_transpose &&
>                strcmp(mat_var->name, "gl_TextureMatrix") == 0) {
> -      ir_dereference_array *array_ref = ir->operands[0]->as_dereference_array();
> -      assert(array_ref != NULL);
> -      ir_dereference_variable *var_ref = array_ref->array->as_dereference_variable();
> -      assert(var_ref && var_ref->var == mat_var);
> -
> -      ir->operands[0] = ir->operands[1];
> -      ir->operands[1] = array_ref;
> -
> -      var_ref->var = texmat_transpose;
> -
> -      texmat_transpose->data.max_array_access =
> -         MAX2(texmat_transpose->data.max_array_access, mat_var->data.max_array_access);
> -
> +      transform_operands_texmat(ir, mat_var, texmat_transpose);
> +      progress = true;
> +   } else if (texmat_inv_transpose &&
> +              strcmp(mat_var->name, "gl_TextureMatrixInverse") == 0) {
> +      transform_operands_texmat(ir, mat_var, texmat_inv_transpose);
>        progress = true;
>     }
>
Hi Ian,

On Mon, 2014-06-16 at 12:20 -0700, Ian Romanick wrote:
> On 04/21/2014 04:38 AM, Iago Toral Quiroga wrote:
> > Currently it only considers the cases of gl_ModelViewProjectionMatrix and
> > gl_TextureMatrix. The same optimization can be done also for
> > gl_ModelViewMatrix, gl_ProjectionMatrix and the corresponding inverses.
> 
> I've been looking at some thing similar recently.  I've been looking at
> flipping user-defined matrices.
> 
> I think rather than adding a bunch of individual handles to flippable
> matrices, I think we should add a hash-table of possible names.  For the
> built-in uniforms, we would add any matrix uniform that doesn't have
> "Transpose" in the name.  Something like:
> 
>     struct matrix_and_transpose {
>         ir_variable *matrix;
>         ir_variable *transpose_matrix;
>     };
> 
> I think all of the names have regular enough patterns that we should be
> able to generate the non-transpose name from the transpose name.  Vice
> versa shouldn't be necessary.  If the transpose name is encountered
> first, add the structure with matrix set NULL and the name derived from
> the transpose name.

Right, I think this is better too.

I understand that you are already working on this then? If not I can
give this a go as soon as I send the v2 of the series for the
multi-stream support (hopefully tomorrow).

Iago

> One other comment below.
> 
> > ---
> >  src/glsl/opt_flip_matrices.cpp | 118 +++++++++++++++++++++++++++++++----------
> >  1 file changed, 91 insertions(+), 27 deletions(-)
> > 
> > diff --git a/src/glsl/opt_flip_matrices.cpp b/src/glsl/opt_flip_matrices.cpp
> > index 9044fd6..bf09b78 100644
> > --- a/src/glsl/opt_flip_matrices.cpp
> > +++ b/src/glsl/opt_flip_matrices.cpp
> > @@ -29,8 +29,9 @@
> >   * On some hardware, this is more efficient.
> >   *
> >   * This currently only does the conversion for built-in matrices which
> > - * already have transposed equivalents.  Namely, gl_ModelViewProjectionMatrix
> > - * and gl_TextureMatrix.
> > + * already have transposed equivalents. Namely, gl_ModelViewMatrix,
> > + * gl_ProjectionMatrix, gl_ModelViewProjectionMatrix, gl_TextureMatrix and
> > + * their inverses.
> >   */
> >  #include "ir.h"
> >  #include "ir_optimization.h"
> > @@ -42,18 +43,39 @@ public:
> >     matrix_flipper(exec_list *instructions)
> >     {
> >        progress = false;
> > +      p_transpose = NULL;
> > +      p_inv_transpose = NULL;
> > +      mv_transpose = NULL;
> > +      mv_inv_transpose = NULL;
> >        mvp_transpose = NULL;
> > +      mvp_inv_transpose = NULL;
> >        texmat_transpose = NULL;
> > +      texmat_inv_transpose = NULL;
> >  
> >        foreach_list(n, instructions) {
> >           ir_instruction *ir = (ir_instruction *) n;
> >           ir_variable *var = ir->as_variable();
> >           if (!var)
> >              continue;
> > -         if (strcmp(var->name, "gl_ModelViewProjectionMatrixTranspose") == 0)
> > +
> > +         if (strcmp(var->name, "gl_ProjectionMatrixTranspose") == 0)
> > +            p_transpose = var;
> > +         else if (strcmp(var->name, "gl_ProjectionMatrixInverseTranspose") == 0)
> > +            p_inv_transpose = var;
> > +         else if (strcmp(var->name, "gl_ModelViewMatrixTranspose") == 0)
> > +            mv_transpose = var;
> > +         else if (strcmp(var->name, "gl_ModelViewMatrixInverseTranspose") == 0)
> > +            mv_inv_transpose = var;
> > +         else if (strcmp(var->name,
> > +                         "gl_ModelViewProjectionMatrixTranspose") == 0)
> >              mvp_transpose = var;
> > -         if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0)
> > +         else if (strcmp(var->name,
> > +                         "gl_ModelViewProjectionMatrixInverseTranspose") == 0)
> > +            mvp_inv_transpose = var;
> > +         else if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0)
> >              texmat_transpose = var;
> > +         else if (strcmp(var->name, "gl_TextureMatrixInverseTranspose") == 0)
> > +            texmat_inv_transpose = var;
> >        }
> >     }
> >  
> > @@ -62,11 +84,49 @@ public:
> >     bool progress;
> >  
> >  private:
> > +   ir_variable *p_transpose;
> > +   ir_variable *p_inv_transpose;
> > +   ir_variable *mv_transpose;
> > +   ir_variable *mv_inv_transpose;
> >     ir_variable *mvp_transpose;
> > +   ir_variable *mvp_inv_transpose;
> >     ir_variable *texmat_transpose;
> > +   ir_variable *texmat_inv_transpose;
> >  };
> >  }
> >  
> > +static void
> > +transform_operands(ir_expression *ir,
> > +                   ir_variable *mat_var, ir_variable *mat_transpose)
> > +{
> > +#ifndef NDEBUG
> > +   ir_dereference_variable *deref = ir->operands[0]->as_dereference_variable();
> > +   assert(deref && deref->var == mat_var);
> > +#endif
> > +
> > +   void *mem_ctx = ralloc_parent(ir);
> > +   ir->operands[0] = ir->operands[1];
> > +   ir->operands[1] = new(mem_ctx) ir_dereference_variable(mat_transpose);
> > +}
> > +
> > +static void
> > +transform_operands_texmat(ir_expression *ir,
> > +                          ir_variable *mat_var, ir_variable *mat_transpose)
> 
> I'd call this transform_operands_array_of_matrix or something.  Once we
> add support for user-defined matrices, this function will be used for
> other arrays too.
> 
> > +{
> > +   ir_dereference_array *array_ref = ir->operands[0]->as_dereference_array();
> > +   assert(array_ref != NULL);
> > +   ir_dereference_variable *var_ref = array_ref->array->as_dereference_variable();
> > +   assert(var_ref && var_ref->var == mat_var);
> > +
> > +   ir->operands[0] = ir->operands[1];
> > +   ir->operands[1] = array_ref;
> > +
> > +   var_ref->var = mat_transpose;
> > +
> > +   mat_transpose->data.max_array_access =
> > +      MAX2(mat_transpose->data.max_array_access, mat_var->data.max_array_access);
> > +}
> > +
> >  ir_visitor_status
> >  matrix_flipper::visit_enter(ir_expression *ir)
> >  {
> > @@ -81,32 +141,36 @@ matrix_flipper::visit_enter(ir_expression *ir)
> >  
> >     if (mvp_transpose &&
> >         strcmp(mat_var->name, "gl_ModelViewProjectionMatrix") == 0) {
> > -#ifndef NDEBUG
> > -      ir_dereference_variable *deref = ir->operands[0]->as_dereference_variable();
> > -      assert(deref && deref->var == mat_var);
> > -#endif
> > -
> > -      void *mem_ctx = ralloc_parent(ir);
> > -
> > -      ir->operands[0] = ir->operands[1];
> > -      ir->operands[1] = new(mem_ctx) ir_dereference_variable(mvp_transpose);
> > -
> > +      transform_operands(ir, mat_var, mvp_transpose);
> > +      progress = true;
> > +   } else if (mvp_inv_transpose &&
> > +              strcmp(mat_var->name,
> > +                     "gl_ModelViewProjectionMatrixInverse") == 0) {
> > +      transform_operands(ir, mat_var, mvp_inv_transpose);
> > +      progress = true;
> > +   } else if (p_transpose &&
> > +              strcmp(mat_var->name, "gl_ProjectionMatrix") == 0) {
> > +      transform_operands(ir, mat_var, p_transpose);
> > +      progress = true;
> > +   } else if (p_inv_transpose &&
> > +              strcmp(mat_var->name, "gl_ProjectionMatrixInverse") == 0) {
> > +      transform_operands(ir, mat_var, p_inv_transpose);
> > +      progress = true;
> > +   } else if (mv_transpose &&
> > +              strcmp(mat_var->name, "gl_ModelViewMatrix") == 0) {
> > +      transform_operands(ir, mat_var, mv_transpose);
> > +      progress = true;
> > +   } else if (mv_inv_transpose &&
> > +              strcmp(mat_var->name, "gl_ModelViewMatrixInverse") == 0) {
> > +      transform_operands(ir, mat_var, mv_inv_transpose);
> >        progress = true;
> >     } else if (texmat_transpose &&
> >                strcmp(mat_var->name, "gl_TextureMatrix") == 0) {
> > -      ir_dereference_array *array_ref = ir->operands[0]->as_dereference_array();
> > -      assert(array_ref != NULL);
> > -      ir_dereference_variable *var_ref = array_ref->array->as_dereference_variable();
> > -      assert(var_ref && var_ref->var == mat_var);
> > -
> > -      ir->operands[0] = ir->operands[1];
> > -      ir->operands[1] = array_ref;
> > -
> > -      var_ref->var = texmat_transpose;
> > -
> > -      texmat_transpose->data.max_array_access =
> > -         MAX2(texmat_transpose->data.max_array_access, mat_var->data.max_array_access);
> > -
> > +      transform_operands_texmat(ir, mat_var, texmat_transpose);
> > +      progress = true;
> > +   } else if (texmat_inv_transpose &&
> > +              strcmp(mat_var->name, "gl_TextureMatrixInverse") == 0) {
> > +      transform_operands_texmat(ir, mat_var, texmat_inv_transpose);
> >        progress = true;
> >     }
> >  
> 
>
On 06/17/2014 05:07 AM, Iago Toral wrote:
> Hi Ian,
> 
> On Mon, 2014-06-16 at 12:20 -0700, Ian Romanick wrote:
>> On 04/21/2014 04:38 AM, Iago Toral Quiroga wrote:
>>> Currently it only considers the cases of gl_ModelViewProjectionMatrix and
>>> gl_TextureMatrix. The same optimization can be done also for
>>> gl_ModelViewMatrix, gl_ProjectionMatrix and the corresponding inverses.
>>
>> I've been looking at some thing similar recently.  I've been looking at
>> flipping user-defined matrices.
>>
>> I think rather than adding a bunch of individual handles to flippable
>> matrices, I think we should add a hash-table of possible names.  For the
>> built-in uniforms, we would add any matrix uniform that doesn't have
>> "Transpose" in the name.  Something like:
>>
>>     struct matrix_and_transpose {
>>         ir_variable *matrix;
>>         ir_variable *transpose_matrix;
>>     };
>>
>> I think all of the names have regular enough patterns that we should be
>> able to generate the non-transpose name from the transpose name.  Vice
>> versa shouldn't be necessary.  If the transpose name is encountered
>> first, add the structure with matrix set NULL and the name derived from
>> the transpose name.
> 
> Right, I think this is better too.
> 
> I understand that you are already working on this then? If not I can
> give this a go as soon as I send the v2 of the series for the
> multi-stream support (hopefully tomorrow).

I was working on something a little different, but I've been sidetracked
by other things.  I probably won't get back to it for a week or more.
It seems like your work is a lot closer to being ready.  I can rebase
mine on top of yours later.

> Iago
> 
>> One other comment below.
>>
>>> ---
>>>  src/glsl/opt_flip_matrices.cpp | 118 +++++++++++++++++++++++++++++++----------
>>>  1 file changed, 91 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/src/glsl/opt_flip_matrices.cpp b/src/glsl/opt_flip_matrices.cpp
>>> index 9044fd6..bf09b78 100644
>>> --- a/src/glsl/opt_flip_matrices.cpp
>>> +++ b/src/glsl/opt_flip_matrices.cpp
>>> @@ -29,8 +29,9 @@
>>>   * On some hardware, this is more efficient.
>>>   *
>>>   * This currently only does the conversion for built-in matrices which
>>> - * already have transposed equivalents.  Namely, gl_ModelViewProjectionMatrix
>>> - * and gl_TextureMatrix.
>>> + * already have transposed equivalents. Namely, gl_ModelViewMatrix,
>>> + * gl_ProjectionMatrix, gl_ModelViewProjectionMatrix, gl_TextureMatrix and
>>> + * their inverses.
>>>   */
>>>  #include "ir.h"
>>>  #include "ir_optimization.h"
>>> @@ -42,18 +43,39 @@ public:
>>>     matrix_flipper(exec_list *instructions)
>>>     {
>>>        progress = false;
>>> +      p_transpose = NULL;
>>> +      p_inv_transpose = NULL;
>>> +      mv_transpose = NULL;
>>> +      mv_inv_transpose = NULL;
>>>        mvp_transpose = NULL;
>>> +      mvp_inv_transpose = NULL;
>>>        texmat_transpose = NULL;
>>> +      texmat_inv_transpose = NULL;
>>>  
>>>        foreach_list(n, instructions) {
>>>           ir_instruction *ir = (ir_instruction *) n;
>>>           ir_variable *var = ir->as_variable();
>>>           if (!var)
>>>              continue;
>>> -         if (strcmp(var->name, "gl_ModelViewProjectionMatrixTranspose") == 0)
>>> +
>>> +         if (strcmp(var->name, "gl_ProjectionMatrixTranspose") == 0)
>>> +            p_transpose = var;
>>> +         else if (strcmp(var->name, "gl_ProjectionMatrixInverseTranspose") == 0)
>>> +            p_inv_transpose = var;
>>> +         else if (strcmp(var->name, "gl_ModelViewMatrixTranspose") == 0)
>>> +            mv_transpose = var;
>>> +         else if (strcmp(var->name, "gl_ModelViewMatrixInverseTranspose") == 0)
>>> +            mv_inv_transpose = var;
>>> +         else if (strcmp(var->name,
>>> +                         "gl_ModelViewProjectionMatrixTranspose") == 0)
>>>              mvp_transpose = var;
>>> -         if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0)
>>> +         else if (strcmp(var->name,
>>> +                         "gl_ModelViewProjectionMatrixInverseTranspose") == 0)
>>> +            mvp_inv_transpose = var;
>>> +         else if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0)
>>>              texmat_transpose = var;
>>> +         else if (strcmp(var->name, "gl_TextureMatrixInverseTranspose") == 0)
>>> +            texmat_inv_transpose = var;
>>>        }
>>>     }
>>>  
>>> @@ -62,11 +84,49 @@ public:
>>>     bool progress;
>>>  
>>>  private:
>>> +   ir_variable *p_transpose;
>>> +   ir_variable *p_inv_transpose;
>>> +   ir_variable *mv_transpose;
>>> +   ir_variable *mv_inv_transpose;
>>>     ir_variable *mvp_transpose;
>>> +   ir_variable *mvp_inv_transpose;
>>>     ir_variable *texmat_transpose;
>>> +   ir_variable *texmat_inv_transpose;
>>>  };
>>>  }
>>>  
>>> +static void
>>> +transform_operands(ir_expression *ir,
>>> +                   ir_variable *mat_var, ir_variable *mat_transpose)
>>> +{
>>> +#ifndef NDEBUG
>>> +   ir_dereference_variable *deref = ir->operands[0]->as_dereference_variable();
>>> +   assert(deref && deref->var == mat_var);
>>> +#endif
>>> +
>>> +   void *mem_ctx = ralloc_parent(ir);
>>> +   ir->operands[0] = ir->operands[1];
>>> +   ir->operands[1] = new(mem_ctx) ir_dereference_variable(mat_transpose);
>>> +}
>>> +
>>> +static void
>>> +transform_operands_texmat(ir_expression *ir,
>>> +                          ir_variable *mat_var, ir_variable *mat_transpose)
>>
>> I'd call this transform_operands_array_of_matrix or something.  Once we
>> add support for user-defined matrices, this function will be used for
>> other arrays too.
>>
>>> +{
>>> +   ir_dereference_array *array_ref = ir->operands[0]->as_dereference_array();
>>> +   assert(array_ref != NULL);
>>> +   ir_dereference_variable *var_ref = array_ref->array->as_dereference_variable();
>>> +   assert(var_ref && var_ref->var == mat_var);
>>> +
>>> +   ir->operands[0] = ir->operands[1];
>>> +   ir->operands[1] = array_ref;
>>> +
>>> +   var_ref->var = mat_transpose;
>>> +
>>> +   mat_transpose->data.max_array_access =
>>> +      MAX2(mat_transpose->data.max_array_access, mat_var->data.max_array_access);
>>> +}
>>> +
>>>  ir_visitor_status
>>>  matrix_flipper::visit_enter(ir_expression *ir)
>>>  {
>>> @@ -81,32 +141,36 @@ matrix_flipper::visit_enter(ir_expression *ir)
>>>  
>>>     if (mvp_transpose &&
>>>         strcmp(mat_var->name, "gl_ModelViewProjectionMatrix") == 0) {
>>> -#ifndef NDEBUG
>>> -      ir_dereference_variable *deref = ir->operands[0]->as_dereference_variable();
>>> -      assert(deref && deref->var == mat_var);
>>> -#endif
>>> -
>>> -      void *mem_ctx = ralloc_parent(ir);
>>> -
>>> -      ir->operands[0] = ir->operands[1];
>>> -      ir->operands[1] = new(mem_ctx) ir_dereference_variable(mvp_transpose);
>>> -
>>> +      transform_operands(ir, mat_var, mvp_transpose);
>>> +      progress = true;
>>> +   } else if (mvp_inv_transpose &&
>>> +              strcmp(mat_var->name,
>>> +                     "gl_ModelViewProjectionMatrixInverse") == 0) {
>>> +      transform_operands(ir, mat_var, mvp_inv_transpose);
>>> +      progress = true;
>>> +   } else if (p_transpose &&
>>> +              strcmp(mat_var->name, "gl_ProjectionMatrix") == 0) {
>>> +      transform_operands(ir, mat_var, p_transpose);
>>> +      progress = true;
>>> +   } else if (p_inv_transpose &&
>>> +              strcmp(mat_var->name, "gl_ProjectionMatrixInverse") == 0) {
>>> +      transform_operands(ir, mat_var, p_inv_transpose);
>>> +      progress = true;
>>> +   } else if (mv_transpose &&
>>> +              strcmp(mat_var->name, "gl_ModelViewMatrix") == 0) {
>>> +      transform_operands(ir, mat_var, mv_transpose);
>>> +      progress = true;
>>> +   } else if (mv_inv_transpose &&
>>> +              strcmp(mat_var->name, "gl_ModelViewMatrixInverse") == 0) {
>>> +      transform_operands(ir, mat_var, mv_inv_transpose);
>>>        progress = true;
>>>     } else if (texmat_transpose &&
>>>                strcmp(mat_var->name, "gl_TextureMatrix") == 0) {
>>> -      ir_dereference_array *array_ref = ir->operands[0]->as_dereference_array();
>>> -      assert(array_ref != NULL);
>>> -      ir_dereference_variable *var_ref = array_ref->array->as_dereference_variable();
>>> -      assert(var_ref && var_ref->var == mat_var);
>>> -
>>> -      ir->operands[0] = ir->operands[1];
>>> -      ir->operands[1] = array_ref;
>>> -
>>> -      var_ref->var = texmat_transpose;
>>> -
>>> -      texmat_transpose->data.max_array_access =
>>> -         MAX2(texmat_transpose->data.max_array_access, mat_var->data.max_array_access);
>>> -
>>> +      transform_operands_texmat(ir, mat_var, texmat_transpose);
>>> +      progress = true;
>>> +   } else if (texmat_inv_transpose &&
>>> +              strcmp(mat_var->name, "gl_TextureMatrixInverse") == 0) {
>>> +      transform_operands_texmat(ir, mat_var, texmat_inv_transpose);
>>>        progress = true;
>>>     }
>>>