[Mesa-dev] [PATCH v4] glsl: Expand matrix flip optimization pass to cover more cases.
Iago Toral Quiroga
itoral at igalia.com
Fri Sep 19 04:39:35 PDT 2014
On vie, 2014-09-19 at 21:16 +1000, Timothy Arceri wrote:
> On Fri, 2014-09-19 at 12:52 +0200, Iago Toral Quiroga wrote:
> > Also, as suggested by Ian Romanick, make it so we don't need a bunch of
> > individual handles to flippable matrices, instead we register
> > matrix/transpose_matrix pairs in a hash table for all built-in matrices
> > using the non-transpose matrix name as key.
> > ---
> > src/glsl/opt_flip_matrices.cpp | 159 +++++++++++++++++++++++++++++++----------
> > 1 file changed, 121 insertions(+), 38 deletions(-)
> >
> > I think this never got the reviewed-by... This is a rebased version of the v3
> > patch that also fixes a silly mistake that I had introduced in that version.
> > No piglit regressions observed on SandyBridge.
> >
> > Ian, do you think this version is good?
> >
> > diff --git a/src/glsl/opt_flip_matrices.cpp b/src/glsl/opt_flip_matrices.cpp
> > index 04c6170..bb449d6 100644
> > --- a/src/glsl/opt_flip_matrices.cpp
> > +++ b/src/glsl/opt_flip_matrices.cpp
> > @@ -29,43 +29,143 @@
> > * 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.
> > */
> > #include "ir.h"
> > #include "ir_optimization.h"
> > #include "main/macros.h"
> > +#include "program/hash_table.h"
> >
> > namespace {
> > class matrix_flipper : public ir_hierarchical_visitor {
> > public:
> > + struct matrix_and_transpose {
> > + ir_variable *matrix;
> > + ir_variable *transpose_matrix;
> > + };
> > +
> > matrix_flipper(exec_list *instructions)
> > {
> > + this->mem_ctx = ralloc_context(NULL);
> > progress = false;
> > - mvp_transpose = NULL;
> > - texmat_transpose = NULL;
> > +
> > + /* Build a hash table of built-in matrices and their transposes.
> > + *
> > + * The key for the entries in the hash table is the non-transpose matrix
> > + * name. This assumes that all built-in transpose matrices have the
> > + * "Transpose" suffix.
> > + */
> > + ht = hash_table_ctor(0, hash_table_string_hash,
> > + hash_table_string_compare);
> >
> > foreach_in_list(ir_instruction, ir, instructions) {
> > ir_variable *var = ir->as_variable();
> > +
> > if (!var)
> > continue;
> > - if (strcmp(var->name, "gl_ModelViewProjectionMatrixTranspose") == 0)
> > - mvp_transpose = var;
> > - if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0)
> > - texmat_transpose = var;
> > +
> > + /* Must be a matrix or array of matrices. */
> > + if (!var->type->is_matrix() &&
> > + !(var->type->is_array() && var->type->fields.array->is_matrix()))
>
>
> This can now be simplified to
>
> if(!var->type->without_array()->is_matrix())
Oh, nice! I have changed the patch to do this instead.
Thanks!
>
> > + continue;
> > +
> > + /* Must be a built-in */
> > + if (!is_gl_identifier(var->name))
> > + continue;
> > +
> > + /* Create a new entry for this matrix if we don't have one yet */
> > + bool new_entry = false;
> > + struct matrix_and_transpose *entry =
> > + (struct matrix_and_transpose *) hash_table_find(ht, var->name);
> > + if (!entry) {
> > + new_entry = true;
> > + entry = new struct matrix_and_transpose();
> > + entry->matrix = NULL;
> > + entry->transpose_matrix = NULL;
> > + }
> > +
> > + const char *transpose_ptr = strstr(var->name, "Transpose");
> > + if (transpose_ptr == NULL) {
> > + entry->matrix = var;
> > + } else {
> > + /* We should not be adding transpose built-in matrices that do
> > + * not end in 'Transpose'.
> > + */
> > + assert(transpose_ptr[9] == 0);
> > + entry->transpose_matrix = var;
> > + }
> > +
> > + if (new_entry) {
> > + char *entry_key;
> > + if (transpose_ptr == NULL) {
> > + entry_key = (char *) var->name;
> > + } else {
> > + entry_key = ralloc_strndup(this->mem_ctx, var->name,
> > + transpose_ptr - var->name);
> > + }
> > + hash_table_insert(ht, entry, entry_key);
> > + }
> > }
> > }
> >
> > + ~matrix_flipper()
> > + {
> > + hash_table_dtor(ht);
> > + ralloc_free(this->mem_ctx);
> > + }
> > +
> > ir_visitor_status visit_enter(ir_expression *ir);
> >
> > bool progress;
> >
> > private:
> > - ir_variable *mvp_transpose;
> > - ir_variable *texmat_transpose;
> > + void transform_operands(ir_expression *ir,
> > + ir_variable *mat_var,
> > + ir_variable *mat_transpose);
> > + void transform_operands_array_of_matrix(ir_expression *ir,
> > + ir_variable *mat_var,
> > + ir_variable *mat_transpose);
> > + struct hash_table *ht;
> > + void *mem_ctx;
> > };
> > }
> >
> > +void
> > +matrix_flipper::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);
> > +}
> > +
> > +void
> > +matrix_flipper::transform_operands_array_of_matrix(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)
> > {
> > @@ -78,37 +178,20 @@ matrix_flipper::visit_enter(ir_expression *ir)
> > if (!mat_var)
> > return visit_continue;
> >
> > - 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);
> > -
> > - 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);
> > + struct matrix_and_transpose *entry =
> > + (struct matrix_and_transpose *) hash_table_find(ht, mat_var->name);
> > + if (!entry || !entry->transpose_matrix)
> > + return visit_continue;
> >
> > - progress = true;
> > + if (strcmp(mat_var->name, "gl_TextureMatrix") == 0 ||
> > + strcmp(mat_var->name, "gl_TextureMatrixInverse") == 0) {
> > + transform_operands_array_of_matrix(ir, mat_var, entry->transpose_matrix);
> > + } else {
> > + transform_operands(ir, mat_var, entry->transpose_matrix);
> > }
> >
> > + progress = true;
> > +
> > return visit_continue;
> > }
> >
>
>
>
More information about the mesa-dev
mailing list