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

[Jason Ekstrand]

Cool, thanks!

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

> > > > > +{
> > > > > +   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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181015/42f22ba6/attachment.html>