On Wed, Apr 4, 2018 at 1:53 PM, Caio Marcelo de Oliveira Filho <
caio.olive...@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!
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
