[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:

> On 02/08/2013 04:24 PM, Eric Anholt wrote:
>> Ian Romanick <idr at freedesktop.org> writes:
>>
>>> From: Ian Romanick <ian.d.romanick at intel.com>
>>> + * 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.
>
> I did it like this so that we could still optimize things like:
>
>      vec4 v = ...;
>
>      v[i] =  ...;   // i is a uniform or similar
>      ...
>      v.x = ...;
>      ...
>      v.y = ...;
>      ...
>      v.z = ...;
>      ...
>      v.w = ...;
>
> Even though the initial assignment(s) set "any field", the later 
> assignments wipe them all out.

But for code that's just:

>      v[i] =  ...;   // i is a uniform or similar
>      ...
>      v.x = ...;

you'll turn off the write_mask of the v[i] write, right?  And now the
read of v.yzw is broken.
-------------- 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/4b3fb1f1/attachment.pgp>