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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
