[Mesa-dev] [PATCH 05/11] nir: Separate dead write removal into its own pass

[Caio Marcelo de Oliveira Filho]

> > > > +{
> > > > +   bool progress = false;
> > > > +
> > > > +   /* Find writes that are unused and can be removed. */
> > > > +   util_dynarray_foreach_reverse(unused_writes, struct write_entry,
> > > > entry) {
> > > > +      nir_deref_compare_result comp = nir_compare_derefs(dst,
> > entry->dst);
> > > > +      if (comp & nir_derefs_a_contains_b_bit) {
> > > >
> > >
> > > Mind throwing an assert in here:
> > >
> > > assert((comp & nir_derefs_equal_bit) || mask ==
> > ~(nir_component_mask_t)0);
> >
> > We can assert that.  We can have an entry for a copy between arrays a
> > and b, and see a store a[1].x that will invalidate the 'x' component
> > of the copy.
> >
> 
> Do you mean, "we can't assert that"?

Correct. I meant "we can't".


> I'm trying to think about whether or not the type of per-component
> invalidation you're talking about there is valid or not.  If we can assume
> that all struct copies are split and that all copies are fully qualified
> (i.e., they end in a vector or scalar with wildcards for all the arrays),
> then I think such inference is fine.  Maybe worth a comment that such is
> intentional?

I've added the following comment and assert to update_unused_writes()

   /* This pass assumes that destination of copies and stores are derefs that
    * end in a vector or scalar (it is OK to have wildcards or indirects for
    * arrays).
    */
   assert(glsl_type_is_vector_or_scalar(dst->type));

My understanding is that in this context this always is true, but in
the future might not be if we do things like: "copy a b" instead of
"copy a[*] b[*]" when a and b are arrays (similar to structs).

Updated my branch with that too.



Caio