[Mesa-stable] [Mesa-dev] [PATCH 2/6] glsl: Split arrays even in the presence of whole-array copies.

[Timothy Arceri]

On Tue, 2016-06-21 at 20:02 -0700, Kenneth Graunke wrote:
> Previously, we failed to split constant arrays.  Code such as
> 
>    int[2] numbers = int[](1, 2);
> 
> would generates a whole-array assignment:
> 
>   (assign () (var_ref numbers)
>              (constant (array int 4) (constant int 1) (constant int
> 2)))
> 
> opt_array_splitting generally tried to visit ir_dereference_array
> nodes,
> and avoid recursing into the inner ir_dereference_variable.  So if it
> ever saw a ir_dereference_variable, it assumed this was a whole-array
> read and bailed.  However, in the above case, there's no array deref,
> and we can totally handle it - we just have to "unroll" the
> assignment,
> creating assignments for each element.
> 
> This was mitigated by the fact that we constant propagate whole
> arrays,
> so a dereference of a single component would usually get the desired
> single value anyway.  However, I plan to stop doing that shortly;
> early experiments with disabling constant propagation of arrays
> revealed this shortcoming.
> 
> This patch causes some arrays in Gl32GSCloth's geometry shaders to be
> split, which allows other optimizations to eliminate unused GS
> inputs.
> The VS then doesn't have to write them, which eliminates the entire
> VS
> (5 -> 2 instructions).  It still renders correctly.
> 
> No other change in shader-db.
> 
> Cc: mesa-stable at lists.freedesktop.org
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/compiler/glsl/opt_array_splitting.cpp | 56
> +++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/src/compiler/glsl/opt_array_splitting.cpp
> b/src/compiler/glsl/opt_array_splitting.cpp
> index a294da5..9faeb87 100644
> --- a/src/compiler/glsl/opt_array_splitting.cpp
> +++ b/src/compiler/glsl/opt_array_splitting.cpp
> @@ -93,6 +93,7 @@ public:
>     {
>        this->mem_ctx = ralloc_context(NULL);
>        this->variable_list.make_empty();
> +      this->in_whole_array_copy = false;
>     }
>  
>     ~ir_array_reference_visitor(void)
> @@ -104,6 +105,8 @@ public:
>  
>     virtual ir_visitor_status visit(ir_variable *);
>     virtual ir_visitor_status visit(ir_dereference_variable *);
> +   virtual ir_visitor_status visit_enter(ir_assignment *);
> +   virtual ir_visitor_status visit_leave(ir_assignment *);
>     virtual ir_visitor_status visit_enter(ir_dereference_array *);
>     virtual ir_visitor_status visit_enter(ir_function_signature *);
>  
> @@ -113,6 +116,8 @@ public:
>     exec_list variable_list;
>  
>     void *mem_ctx;
> +
> +   bool in_whole_array_copy;
>  };
>  
>  } /* namespace */
> @@ -158,10 +163,34 @@ ir_array_reference_visitor::visit(ir_variable
> *ir)
>  }
>  
>  ir_visitor_status
> +ir_array_reference_visitor::visit_enter(ir_assignment *ir)
> +{
> +   in_whole_array_copy =
> +      ir->lhs->type->is_array() && !ir->lhs->type-
> >is_array_of_arrays() &&
> +      ir->whole_variable_written();

Maybe a TODO for AoA support? I assume we would just need to do some
kind of recersive call in the new code below or is there more to it? If
there is more to it?

> +
> +   return visit_continue;
> +}
> +
> +ir_visitor_status
> +ir_array_reference_visitor::visit_leave(ir_assignment *ir)
> +{
> +   in_whole_array_copy = false;
> +
> +   return visit_continue;
> +}
> +
> +ir_visitor_status
>  ir_array_reference_visitor::visit(ir_dereference_variable *ir)
>  {
>     variable_entry *entry = this->get_variable_entry(ir->var);
>  
> +   /* Ignore whole-array assignments on the LHS.  We can split those
> +    * by "unrolling" the assignment into component-wise assignments.
> +    */

Instead of ignore maybe "Allow" or "Allow splitting of" or something
like that I think makes it easier to understand whats going on.

Otherwise patch 1-2 are:

Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>

> +   if (in_assignee && in_whole_array_copy)
> +      return visit_continue;
> +
>     /* If we made it to here without seeing an ir_dereference_array,
>      * then the dereference of this array didn't have a constant
> index
>      * (see the visit_continue_with_parent below), so we can't split
> @@ -350,6 +379,33 @@
> ir_array_splitting_visitor::visit_leave(ir_assignment *ir)
>      */
>     ir_rvalue *lhs = ir->lhs;
>  
> +   /* "Unroll" any whole array assignments, creating assignments for
> +    * each array element.  Then, do splitting on each new
> assignment.
> +    */
> +   if (lhs->type->is_array() && ir->whole_variable_written() &&
> +       get_splitting_entry(ir->whole_variable_written())) {
> +      void *mem_ctx = ralloc_parent(ir);
> +
> +      for (unsigned i = 0; i < lhs->type->length; i++) {
> +         ir_rvalue *lhs_i =
> +            new(mem_ctx) ir_dereference_array(ir->lhs-
> >clone(mem_ctx, NULL),
> +                                              new(mem_ctx)
> ir_constant(i));
> +         ir_rvalue *rhs_i =
> +            new(mem_ctx) ir_dereference_array(ir->rhs-
> >clone(mem_ctx, NULL),
> +                                              new(mem_ctx)
> ir_constant(i));
> +         ir_rvalue *condition_i =
> +            ir->condition ? ir->condition->clone(mem_ctx, NULL) :
> NULL;
> +
> +         ir_assignment *assign_i =
> +            new(mem_ctx) ir_assignment(lhs_i, rhs_i, condition_i);
> +
> +         ir->insert_before(assign_i);
> +         assign_i->accept(this);
> +      }
> +      ir->remove();
> +      return visit_continue;
> +   }
> +
>     handle_rvalue(&lhs);
>     ir->lhs = lhs->as_dereference();
>