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

[Ian Romanick]

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;
>     }
>