[Mesa-dev] [PATCH v3 013/104] nir: Support deref instructions in remove_dead_variables

[Jason Ekstrand]

On Wed, Apr 4, 2018 at 1:53 PM, Caio Marcelo de Oliveira Filho <
caio.oliveira at intel.com> wrote:

> Hi,
>
> > @@ -144,6 +197,45 @@ remove_dead_var_writes(nir_shader *shader, struct
> set *live)
> >                 nir_instr_remove(instr);
> >           }
> >        }
> > +
> > +      /* We walk the list of instructions backwards because we're going
> to
> > +       * delete a deref and all of it's uses and we don't want to end up
> > +       * deleting stuff ahead of us.
> > +       */
> > +      nir_foreach_block(block, function->impl) {
> > +         nir_foreach_instr_safe(instr, block) {
>
> The comment says backwards, the loop walks forwards.
>
> It seems to me propagating the mode needs to be forward, e.g. a deref
> will be marked mode = 0 because of a variable, then another deref that
> has the first as parent marked mode = 0. But I might be missing
> something.
>

Nope.  Just rebase fail on my part.  I'll drop the comment.


> > +            switch (instr->type) {
> > +            case nir_instr_type_deref: {
> > +               /* For deref instructions, propagate modes */
> > +               nir_deref_instr *deref = nir_instr_as_deref(instr);
> > +               if (deref->deref_type == nir_deref_type_var) {
> > +                  deref->mode = deref->var->data.mode;
> > +               } else {
> > +                  nir_deref_instr *parent =
> nir_deref_instr_parent(deref);
> > +                  deref->mode = parent->mode;
> > +               }
>
> Should we write deref->mode only if the new mode is zero?
> I.e. deref->var->data.mode == 0 for the first case, and parent->mode
> == 0 for the else case.
>

In this case, the mode will either match or deref->var->data.mode will be
0.  I'm happy to add an if just to ensure we don't do anything we dind't
intend to.

Thanks for reviewing!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180404/03de67e7/attachment.html>