[Mesa-dev] [PATCH V2] glsl: remove dead code in a single pass

Timothy Arceri t_arceri at yahoo.com.au
Sun Jul 12 02:23:47 PDT 2015


On Sun, 2015-07-12 at 15:47 +1000, Timothy Arceri wrote:
> Currently only one ir assignment is removed for each var in a single
> dead code optimisation pass. This means if a var has more than one
> assignment, then it requires all the glsl optimisations to be run again
> for each additional assignment to be removed.
> Another pass is also required to remove the variable itself.
> 
> With this change all assignments and the variable are removed in a single
> pass.
> 
> Some of the arrays of arrays conformance tests that were looping
> through 8 dimensions ended up with a var with hundreds of assignments.
> 
> This change helps ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1
> go from around 3 min 20 sec -> 2 min
> 
> ES31-CTS.arrays_of_arrays.InteractionFunctionCalls2 went from
> around 9 min 20 sec to 7 min 30 sec
> 
> I had difficulty getting the public shader-db to give a constent result
> with or without this change but the results seemed unchanged at between
> 15-20 seconds.
> 
> V2: Dont add assignment ir to the list if its declaration is out of
> scope.

On second thought I'm not 100% sure this is safe to check like this in the
refcount visitor I'll probably drop this version 2 and just stick with the
original if people think its ok.

>  Add assert to be sure references are counted before assignments.
> ---
>  It would be great if someone could check this against the larger shader-db.
> 
>  The final place where there tests are getting stuck is in constant folding
>  and propagation.
> 
>  src/glsl/ir_variable_refcount.cpp | 29 ++++++++++++++++++++++++++---
>  src/glsl/ir_variable_refcount.h   | 13 ++++++++++++-
>  src/glsl/opt_dead_code.cpp        | 30 +++++++++++++++++++++---------
>  src/glsl/opt_tree_grafting.cpp    |  2 --
>  4 files changed, 59 insertions(+), 15 deletions(-)
> 
> diff --git a/src/glsl/ir_variable_refcount.cpp 
> b/src/glsl/ir_variable_refcount.cpp
> index e4d825c..46eb2ca 100644
> --- a/src/glsl/ir_variable_refcount.cpp
> +++ b/src/glsl/ir_variable_refcount.cpp
> @@ -46,6 +46,16 @@ static void
>  free_entry(struct hash_entry *entry)
>  {
>     ir_variable_refcount_entry *ivre = (ir_variable_refcount_entry *) entry
> ->data;
> +
> +   /* Free assignment list */
> +   while (!ivre->assign_list.is_empty()) {
> +      struct assignment_entry *assignment_entry =
> +         exec_node_data(struct assignment_entry,
> +                        ivre->assign_list.head, link);
> +      assignment_entry->link.remove();
> +      free(assignment_entry);
> +   }
> +
>     delete ivre;
>  }
>  
> @@ -59,7 +69,6 @@ 
> ir_variable_refcount_visitor::~ir_variable_refcount_visitor()
>  ir_variable_refcount_entry::ir_variable_refcount_entry(ir_variable *var)
>  {
>     this->var = var;
> -   assign = NULL;
>     assigned_count = 0;
>     declaration = false;
>     referenced_count = 0;
> @@ -125,8 +134,22 @@ ir_variable_refcount_visitor::visit_leave(ir_assignment 
> *ir)
>     entry = this->get_variable_entry(ir->lhs->variable_referenced());
>     if (entry) {
>        entry->assigned_count++;
> -      if (entry->assign == NULL)
> -      entry->assign = ir;
> +
> +      /**
> +       * Build a list for dead code optimisation. Dont add assingment if it
> +       * was declared out of scope (outside the instruction stream). Also 
> dont
> +       * bother adding any more to the list if there are more references 
> then
> +       * assignments as this means the variable is used and won't be 
> optimised
> +       * out.
> +       */
> +      assert(entry->referenced_count >= entry->assigned_count);
> +      if (entry->declaration &&
> +          entry->referenced_count == entry->assigned_count) {
> +         struct assignment_entry *assignment_entry =
> +            (struct assignment_entry *)calloc(1, 
> sizeof(*assignment_entry));
> +         assignment_entry->assign = ir;
> +         entry->assign_list.push_head(&assignment_entry->link);
> +      }
>     }
>  
>     return visit_continue;
> diff --git a/src/glsl/ir_variable_refcount.h 
> b/src/glsl/ir_variable_refcount.h
> index c15e8110..5c74c31 100644
> --- a/src/glsl/ir_variable_refcount.h
> +++ b/src/glsl/ir_variable_refcount.h
> @@ -33,13 +33,24 @@
>  #include "ir_visitor.h"
>  #include "glsl_types.h"
>  
> +struct assignment_entry {
> +   exec_node link;
> +   ir_assignment *assign;
> +};
> +
>  class ir_variable_refcount_entry
>  {
>  public:
>     ir_variable_refcount_entry(ir_variable *var);
>  
>     ir_variable *var; /* The key: the variable's pointer. */
> -   ir_assignment *assign; /* An assignment to the variable, if any */
> +
> +   /**
> +    * List of assignments to the variable, if any.
> +    * This is intended to be used for dead code optimisation and may
> +    * not be a complete list.
> +    */
> +   exec_list assign_list;
>  
>     /** Number of times the variable is referenced, including assignments. 
> */
>     unsigned referenced_count;
> diff --git a/src/glsl/opt_dead_code.cpp b/src/glsl/opt_dead_code.cpp
> index f45bf5d..6d1f675 100644
> --- a/src/glsl/opt_dead_code.cpp
> +++ b/src/glsl/opt_dead_code.cpp
> @@ -75,22 +75,34 @@ do_dead_code(exec_list *instructions, bool 
> uniform_locations_assigned)
>         || !entry->declaration)
>        continue;
>  
> -      if (entry->assign) {
> -      /* Remove a single dead assignment to the variable we found.
> +      if (!entry->assign_list.is_empty()) {
> +      /* Remove all the dead assignments to the variable we found.
>         * Don't do so if it's a shader or function output, though.
>         */
>        if (entry->var->data.mode != ir_var_function_out &&
>            entry->var->data.mode != ir_var_function_inout &&
>               entry->var->data.mode != ir_var_shader_out) {
> -         entry->assign->remove();
> -         progress = true;
>  
> -         if (debug) {
> -            printf("Removed assignment to %s@%p\n",
> -                   entry->var->name, (void *) entry->var);
> -         }
> +            while (!entry->assign_list.is_empty()) {
> +               struct assignment_entry *assignment_entry =
> +                  exec_node_data(struct assignment_entry,
> +                                 entry->assign_list.head, link);
> +
> +            assignment_entry->assign->remove();
> +
> +            if (debug) {
> +               printf("Removed assignment to %s@%p\n",
> +                      entry->var->name, (void *) entry->var);
> +               }
> +
> +               assignment_entry->link.remove();
> +               free(assignment_entry);
> +            }
> +            progress = true;
>        }
> -      } else {
> +      }
> +
> +      if (entry->assign_list.is_empty()) {
>        /* If there are no assignments or references to the variable left,
>         * then we can remove its declaration.
>         */
> diff --git a/src/glsl/opt_tree_grafting.cpp b/src/glsl/opt_tree_grafting.cpp
> index d47613c..cda6447 100644
> --- a/src/glsl/opt_tree_grafting.cpp
> +++ b/src/glsl/opt_tree_grafting.cpp
> @@ -371,8 +371,6 @@ tree_grafting_basic_block(ir_instruction *bb_first,
>         entry->referenced_count != 2)
>        continue;
>  
> -      assert(assign == entry->assign);
> -
>        /* Found a possibly graftable assignment.  Now, walk through the
>         * rest of the BB seeing if the deref is here, and if nothing 
> interfered with
>         * pasting its expression's values in between.


More information about the mesa-dev mailing list