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

[Jason Ekstrand]

Latest version is

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

On Mon, Oct 15, 2018 at 2:41 PM Jason Ekstrand <jason at jlekstrand.net> wrote:

> 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/865c318e/attachment-0001.html>