On 09/28/2015 07:42 PM, 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 consistent result
> with or without this change but the results seemed unchanged at between
> 15-20 seconds.
>
> Thomas Helland measured change with shader-db on his machine from
> approx 117 secs to 112 secs.
>
> V3: Simplify freeing of list as suggested by Ian, and spelling fixes.
>
> V2: Add assert to be sure references are counted before assignments.
>
> Tested-By: Thomas Helland <thomashellan...@gmail.com>
> ---
> src/glsl/ir_variable_refcount.cpp | 27 ++++++++++++++++++++++++---
> src/glsl/ir_variable_refcount.h | 13 ++++++++++++-
> src/glsl/opt_dead_code.cpp | 33 ++++++++++++++++++++++-----------
> src/glsl/opt_tree_grafting.cpp | 2 --
> 4 files changed, 58 insertions(+), 17 deletions(-)
>
> diff --git a/src/glsl/ir_variable_refcount.cpp
> b/src/glsl/ir_variable_refcount.cpp
> index e4d825c..c8a5575 100644
> --- a/src/glsl/ir_variable_refcount.cpp
> +++ b/src/glsl/ir_variable_refcount.cpp
> @@ -46,6 +46,15 @@ static void
> free_entry(struct hash_entry *entry)
> {
> ir_variable_refcount_entry *ivre = (ir_variable_refcount_entry *)
> entry->data;
> +
> + /* Free assignment list */
> + exec_node *n;
> + while ((n = ivre->assign_list.pop_head()) != NULL) {
> + struct assignment_entry *assignment_entry =
> + exec_node_data(struct assignment_entry, n, link);
> + free(assignment_entry);
> + }
> +
> delete ivre;
> }
>
> @@ -59,7 +68,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 +133,21 @@ 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;
> +
> + /**
This is not a doxygen comment. :) With that fixed, this patch is
Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
I also ran it through our CI system, so
Tested-by: Ian Romanick <ian.d.roman...@intel.com>
> + * Build a list for dead code optimisation. Don't add assignment if it
> + * was declared out of scope (outside the instruction stream). Also
> don't
> + * bother adding any more to the list if there are more references than
> + * assignments as this means the variable is used and won't be
> optimised
> + * out.
> + */
> + assert(entry->referenced_count >= entry->assigned_count);
> + if (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 2cb7f41..4fa259c 100644
> --- a/src/glsl/opt_dead_code.cpp
> +++ b/src/glsl/opt_dead_code.cpp
> @@ -75,24 +75,35 @@ 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.
> - * Don't do so if it's a shader or function output or a shader
> - * storage variable though.
> + 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->var->data.mode != ir_var_shader_storage) {
> - 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 a7a219c..e38a0e9 100644
> --- a/src/glsl/opt_tree_grafting.cpp
> +++ b/src/glsl/opt_tree_grafting.cpp
> @@ -373,8 +373,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.
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
