<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 17, 2014 at 6:01 PM, Connor Abbott <span dir="ltr"><<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
> +<br>
> +static nir_deref *<br>
> +get_deref_tail(nir_deref *deref)<br>
> +{<br>
> +   while (deref->child != NULL)<br>
> +      deref = deref->child;<br>
> +   return deref;<br>
> +}<br>
<br>
</div></div>I think long-term, we should probably make dereferences exec lists so<br>
we don't have to do things like this. In the meantime, though, I<br>
could've sworn I had to write this exact same function somewhere else<br>
(glsl to nir?)... might be a good idea to move this to nir.c and reuse<br>
it, unless nobody else needs this.<br></blockquote><div><br></div><div>I also thought about making them a single block of data and dropping the linked list thing entirely.  It always has to start with a variable and the only difference between an array deref and a structure deref is that an array deref can have an indirect/wildcard.  This would make it a little more awkward to build the deref chains, but they would take less memory and copying/iterating would be easier.  That said, I haven't written the patch yet, so I don't know how hard it would be.<br><br></div><div>And yes, I copied that function from lower_variables_scalar.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> +<br>
> +static void<br>
> +nir_split_var_copy_instr(nir_intrinsic_instr *old_copy,<br>
> +                         nir_deref *dest_head, nir_deref *src_head,<br>
> +                         nir_deref *dest_tail, nir_deref *src_tail,<br>
> +                         struct split_var_copies_state *state)<br>
<br>
</span>It took me a while to see exactly what this is doing, why we need<br>
separate dest_tail and src_tail variables even though they're almost<br>
always the same, and how it creates this temporary deref chain that's<br>
shared between source and destination (a no-no in NIR) but that's ok<br>
because they both get copied at the very end of the recursion when we<br>
actually emit the instructions. I'd like to see a comment explaining<br>
how this function works that explains those things.<br></blockquote><div><br></div><div>Sure.  A comment would be a good plan.  Also, I do modify deref chains in several of my patches.  Sometimes I even set them back.  Yes, this is probably bad form.  However, the kind of iteration we're doing almost requires it.  I'd like to come up with a better plan for that at some point, but I haven't found one yet.<br></div>--Jason<br></div></div></div>