[Mesa-dev] [PATCH] glsl/constant propagation: kill whole var if LHS involves array indexing.

[Ian Romanick]

On 05/19/2012 10:08 AM, Paul Berry wrote:
> When considering which components of a variable were killed by an
> assignment, constant propagation would previously just use the write
> mask of the assignment.  This worked if the LHS of the assignment was
> simple, e.g.:
>
> v.xy = ...; // (assign (xy) (var_ref v) ...)
>
> But it did the wrong thing if the LHS of the assignment involved an
> array indexing operator, since in this case the write mask is always
> (x):
>
> v[i] = ...; // (assign (x) (deref_array (var_ref v) (var_ref i)) ...)
>
> In general, we can't predict which vector component will be selected
> by array indexing, so the only safe thing to do in this case is to
> kill the entire variable.
>
> Fixes piglit tests {fs,vs}-vector-indexing-kills-all-channels.shader_test.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> ---
>   src/glsl/opt_constant_propagation.cpp |   21 ++++++++++++++++++++-
>   1 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/src/glsl/opt_constant_propagation.cpp b/src/glsl/opt_constant_propagation.cpp
> index d381263..2601b52 100644
> --- a/src/glsl/opt_constant_propagation.cpp
> +++ b/src/glsl/opt_constant_propagation.cpp
> @@ -241,7 +241,26 @@ ir_constant_propagation_visitor::visit_leave(ir_assignment *ir)
>      if (this->in_assignee)
>         return visit_continue;
>
> -   kill(ir->lhs->variable_referenced(), ir->write_mask);
> +   unsigned kill_mask = ir->write_mask;
> +   if (ir->lhs->as_dereference_array()) {
> +      /* The LHS of the assignment uses an array indexing operator (e.g. v[i]
> +       * = ...;).  Since we only try to constant propagate vectors and
> +       * scalars, this means that either (a) array indexing is being used to
> +       * select a vector component, or (b) the variable in question is neither
> +       * a scalar or a vector, so we don't care about it.  In the former case,
> +       * we want to kill the whole vector, since in general we can't predict
> +       * which vector component will be selected by array indexing.  In the
> +       * latter case, it doesn't matter what we do, so go ahead and kill the
> +       * whole variable anyway.
> +       *
> +       * Note that if the array index is constant (e.g. v[2] = ...;), we could
> +       * in principle be smarter, but we don't need to, because a future
> +       * optimization pass will convert it to a simple assignment with the
> +       * correct mask.
> +       */
> +      kill_mask = ~0;
> +   }
> +   kill(ir->lhs->variable_referenced(), kill_mask);
>
>      add_constant(ir);
>