[Mesa-dev] [PATCH 18/22] glsl: Move 'foo = foo; ' optimization to opt_dead_code_local

[Alejandro Piñeiro]

On 21/09/17 16:34, Ian Romanick wrote:
> From: "\"Ian Romanick\"" <idr at freedesktop.org>
>
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> The optimization as done in opt_copy_propagation would have to be
> removed in the next patch, and doing so would lead to some substantial
> regressions.

Based on the cover-letter, I guess that are "regressions on shader-db
numbers, even on platforms that use NIR". Perhaps it is worth to mention
on the commit message?

>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Jason Ekstrand <jason at jlekstrand.net>
> ---
>  src/compiler/glsl/opt_copy_propagation.cpp | 19 +++++++------------
>  src/compiler/glsl/opt_dead_code_local.cpp  | 11 +++++++++++
>  2 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/src/compiler/glsl/opt_copy_propagation.cpp b/src/compiler/glsl/opt_copy_propagation.cpp
> index b72ae5a..b8ef0de0 100644
> --- a/src/compiler/glsl/opt_copy_propagation.cpp
> +++ b/src/compiler/glsl/opt_copy_propagation.cpp
> @@ -348,18 +348,13 @@ ir_copy_propagation_visitor::add_copy(ir_assignment *ir)
>     ir_variable *lhs_var = ir->whole_variable_written();
>     ir_variable *rhs_var = ir->rhs->whole_variable_referenced();
>  
> -   if ((lhs_var != NULL) && (rhs_var != NULL)) {
> -      if (lhs_var == rhs_var) {
> -	 /* This is a dumb assignment, but we've conveniently noticed
> -	  * it here.  Removing it now would mess up the loop iteration
> -	  * calling us.  Just flag it to not execute, and someone else
> -	  * will clean up the mess.
> -	  */
> -	 ir->condition = new(ralloc_parent(ir)) ir_constant(false);
> -	 this->progress = true;
> -      } else if (lhs_var->data.mode != ir_var_shader_storage &&
> -                 lhs_var->data.mode != ir_var_shader_shared &&
> -                 lhs_var->data.precise == rhs_var->data.precise) {
> +   /* Don't try to remove a dumb assignment of a variable to itself.  Removing
> +    * it now would mess up the loop iteration calling us.
> +    */
> +   if (lhs_var != NULL && rhs_var != NULL && lhs_var != rhs_var) {
> +      if (lhs_var->data.mode != ir_var_shader_storage &&
> +          lhs_var->data.mode != ir_var_shader_shared &&
> +          lhs_var->data.precise == rhs_var->data.precise) {
>           assert(lhs_var);
>           assert(rhs_var);
>           _mesa_hash_table_insert(acp, lhs_var, rhs_var);
> diff --git a/src/compiler/glsl/opt_dead_code_local.cpp b/src/compiler/glsl/opt_dead_code_local.cpp
> index a403879..3cbc441 100644
> --- a/src/compiler/glsl/opt_dead_code_local.cpp
> +++ b/src/compiler/glsl/opt_dead_code_local.cpp
> @@ -173,6 +173,17 @@ process_assignment(void *lin_ctx, ir_assignment *ir, exec_list *assignments)
>     bool progress = false;
>     kill_for_derefs_visitor v(assignments);
>  
> +   if (ir->condition == NULL) {
> +      /* If this is an assignment of the form "foo = foo;", remove the whole
> +       * instruction and be done with it.
> +       */
> +      const ir_variable *const lhs_var = ir->whole_variable_written();
> +      if (lhs_var != NULL && lhs_var == ir->rhs->whole_variable_referenced()) {
> +         ir->remove();
> +         return true;
> +      }
> +   }
> +
>     /* Kill assignment entries for things used to produce this assignment. */
>     ir->rhs->accept(&v);
>     if (ir->condition) {