From 410609c9688d3f6ad808d33928a7d9589a708a40 Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Sat, 11 Jul 2015 21:38:54 +1000 Subject: [PATCH] glsl: remove dead code in a single pass 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. Reviewed-by: Ian Romanick Tested-By: Thomas Helland Tested-by: Ian Romanick --- src/glsl/ir_variable_refcount.cpp | 26 +++++++++++++++++++++--- src/glsl/ir_variable_refcount.h | 13 +++++++++++- src/glsl/opt_dead_code.cpp | 33 ++++++++++++++++++++----------- src/glsl/opt_tree_grafting.cpp | 2 -- 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/glsl/ir_variable_refcount.cpp b/src/glsl/ir_variable_refcount.cpp index e4d825c454b..790627bd1e3 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,20 @@ 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. 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 c15e8110d04..5c74c314781 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 071485ad31b..c5be166e75a 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 a7a219c55ca..e38a0e93058 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. -- 2.30.2