[Mesa-dev] [PATCH] glsl: Fix handling of array dereferences of vectors in opt_dead_code_local

[Eric Anholt]

Ian Romanick <idr at freedesktop.org> writes:

> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Three parts to the fix:
>
> 1. If the array dereference is a constant index, figure out the real
> write mask.
>
> 2. If the array dereference not is a constant index, assume the
> assignment may generate any component.
>
> 3. If the array dereference not is a constant index, assume the
> assigment will not kill any previous component write.

Man I wish we didn't leave vector array derefs in the GLSL IR.  Or
rather, I wish we didn't try optimizing on this IR.

> -static bool debug = false;
> +static bool debug = true;

left the debug on.

>  
>  namespace {
>  
> +static bool
> +is_array_deref_of_vector(ir_rvalue *ir)
> +{
> +   if (ir->ir_type == ir_type_dereference_array) {
> +      ir_dereference_array *const d = ir->as_dereference_array();
> +
> +      assert(d != NULL);

Or just make the if statement "if (d)" and drop the assert.

> + * a vector, the \c write_mask of the assignment is returned.  If the LHS of
> + * the assignment is an array dereference, the write mask is a lie.  The
> + * actual write mask is determined by the array index.  If the array index is
> + * a constant, calculate the mask from that constant.

> + * If the index is not a constant, the function can operation in one of two

                                                       operate

> + * modes.  In the first mode, when \c want_availability_mask is \c true, it is
> + * assumed that any field of the vector may be written.  This is used when
> + * adding an assignment to the assignment list.  In the second mode, when
> + * \c want_availability_mask is \c false, it is assumed that no fields of the
> + * vector are written.  This is used when trying to remove earlier assignments
> + * from the list.
> + */
> +static int
> +may_set_mask(ir_assignment *ir, bool want_availability_mask)
> +{
> +   int mask = ir->write_mask;
> +
> +   /* If the LHS is an array derefernce of a vector, try to figure out what
> +    * the real write mask is.  If the index is not a constant, assume that any
> +    * element may be written.
> +    */
> +   if (is_array_deref_of_vector(ir->lhs)) {
> +      ir_dereference_array *const d = ir->lhs->as_dereference_array();
> +      ir_constant *const c = d->array_index->as_constant();
> +
> +      if (c != NULL) {
> +         const int idx = (c != NULL) ? c->get_uint_component(0) : -1;
> +
> +         if (idx >= 0 && idx <= 3)
> +            mask = 1U << idx;
> +         else
> +            mask = 0;
> +      } else {
> +         /* Set the write-mask depending on the size of the vector.
> +          */
> +         if (want_availability_mask)
> +            mask = (1U << d->array->type->vector_components()) - 1;
> +         else
> +            mask = 0;
> +      }
> +   }
> +
> +   return mask;
> +}


> +
>  class assignment_entry : public exec_node
>  {
>  public:
> @@ -51,7 +114,7 @@ public:
>        assert(ir);
>        this->lhs = lhs;
>        this->ir = ir;
> -      this->available = ir->write_mask;
> +      this->available = may_set_mask(ir, true);

I don't think the want_availability_mask makes sense. If you're noting
an array deref of a vector for later dead code elimination, a later
instruction that sets the vector's .x shouldn't go turn off .x of this
instruction.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130208/682e615e/attachment.pgp>