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

[Jason Ekstrand]

On Mon, Oct 15, 2018 at 12:37 PM Caio Marcelo de Oliveira Filho <
caio.oliveira at intel.com> wrote:

> Hi,
>
> > > +{
> > > +   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"?

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?

(...)
>
> > > +      case nir_intrinsic_copy_deref: {
> > > +         nir_deref_instr *src = nir_src_as_deref(intrin->src[1]);
> > > +         nir_deref_instr *dst = nir_src_as_deref(intrin->src[0]);
> > > +
> > > +         /* Self-copy is removed. */
> > > +         if (nir_compare_derefs(src, dst) & nir_derefs_equal_bit) {
> > > +            nir_instr_remove(instr);
> > > +            progress = true;
> > > +            break;
> > > +         }
> > > +
> > > +         uintptr_t mask = ~(1 << NIR_MAX_VEC_COMPONENTS);
> > >
> >
> > I don't think this does quite what you want.  Perhaps
> >
> > nir_component_mask_t mask = ~(nir_component_mask_t)0;
>
> I'm going with
>
> nir_component_mask_t mask = (1 << glsl_get_vector_elements(dst->type)) - 1;
>
>
> The idea is that we only fill bits that are valid, so we can detect
> the condition that no bits are set and remove the entry.  Sounds good?
>

Seems reasonable.  Again, this assumes that dst-type is a vector or scalar
and not a struct, array, or other odd type.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181015/38dba8ed/attachment.html>