<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Apr 4, 2018 at 1:53 PM, Caio Marcelo de Oliveira Filho <span dir="ltr"><<a href="mailto:caio.oliveira@intel.com" target="_blank">caio.oliveira@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<span class=""><br>
> @@ -144,6 +197,45 @@ remove_dead_var_writes(nir_<wbr>shader *shader, struct set *live)<br>
>                 nir_instr_remove(instr);<br>
>           }<br>
>        }<br>
> +<br>
> +      /* We walk the list of instructions backwards because we're going to<br>
> +       * delete a deref and all of it's uses and we don't want to end up<br>
> +       * deleting stuff ahead of us.<br>
> +       */<br>
> +      nir_foreach_block(block, function->impl) {<br>
> +         nir_foreach_instr_safe(instr, block) {<br>
<br>
</span>The comment says backwards, the loop walks forwards.<br>
<br>
It seems to me propagating the mode needs to be forward, e.g. a deref<br>
will be marked mode = 0 because of a variable, then another deref that<br>
has the first as parent marked mode = 0. But I might be missing<br>
something.<span class=""><br></span></blockquote><div><br></div><div>Nope.  Just rebase fail on my part.  I'll drop the comment.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> +            switch (instr->type) {<br>
> +            case nir_instr_type_deref: {<br>
> +               /* For deref instructions, propagate modes */<br>
> +               nir_deref_instr *deref = nir_instr_as_deref(instr);<br>
> +               if (deref->deref_type == nir_deref_type_var) {<br>
> +                  deref->mode = deref->var->data.mode;<br>
> +               } else {<br>
> +                  nir_deref_instr *parent = nir_deref_instr_parent(deref);<br>
> +                  deref->mode = parent->mode;<br>
> +               }<br>
<br>
</span>Should we write deref->mode only if the new mode is zero?<br>
I.e. deref->var->data.mode == 0 for the first case, and parent->mode<br>
== 0 for the else case.<br></blockquote><div><br></div><div>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.<br><br></div><div>Thanks for reviewing! <br></div></div></div></div>