<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Dec 15, 2016 at 6:29 PM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto"><span class="gmail-"><div><div class="gmail_extra"><div class="gmail_quote">On Dec 15, 2016 3:08 PM, "Eric Anholt" <<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>> wrote:<br type="attribution"><blockquote class="gmail-m_4779546937062807456quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-m_4779546937062807456quoted-text">Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> writes:<br>
<br>
> This keeps some of Connor's original code.  However, while I was at it,<br>
> I updated this very old pass to a bit more modern NIR.<br>
<br>
</div>I love how much smaller this code is.  Assuming that the phi builder<br>
does that job correctly, this is pretty trivial to read and understand.<br></blockquote></div></div></div><div dir="auto"><br></div></span><div dir="auto">The bigger vars_to_ssa has been using the phi builder for some time now and it's handled everything we've ever thrown at it.</div><span class="gmail-"><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail-m_4779546937062807456quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
The only things I saw were:<br>
<br>
- It doesn't call nir_metadata_preserve() like the old pass did, while<br>
  it adds new SSA values.  Seems like it should.<br></blockquote></div></div></div><div dir="auto"><br></div></span><div dir="auto">Yup.   I'll get that fixed.</div><div dir="auto"><br></div><div dir="auto"><span class="gmail-"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail-m_4779546937062807456quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- It asserts that reg->num_array_elems == 0, while the old pass would<br>
  just skip those.  It looks like the only source of arrays would be<br>
  nir_lower_locals_to_regs(), which only i965 is calling.  Is that<br>
  something we should be using?  Should we be handling the array reg<br>
  case in this pass and in our drivers?<br>
</blockquote></div><br></div></span><div class="gmail_extra" dir="auto">I've got a patch for that.  I needed it for my hack testing.  I'll as it to the pass.  Incidentally, I don't think the old pass handles either of those correctly, it just assumed everything was a "normal" register.</div></div></div>
</blockquote></div><br></div><div class="gmail_extra">I just sent a v3 with all of the comments addressed.  It's currently running on Jenkins with a hack patch that, instead of just going out of SSA, goes out of SSA completely, calls regs_to_ssa to go back in, and then goes out of SSA as normal.  I think it's pretty well tested now. :)<br><br><a href="https://cgit.freedesktop.org/~jekstrand/mesa/commit/?h=jenkins_gl">https://cgit.freedesktop.org/~jekstrand/mesa/commit/?h=jenkins_gl</a><br></div></div>