[Mesa-dev] [PATCH 090/133] nir: Add a copy splitting pass
Jason Ekstrand
jason at jlekstrand.net
Wed Dec 17 18:38:16 PST 2014
On Wed, Dec 17, 2014 at 6:01 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> > +
> > +static nir_deref *
> > +get_deref_tail(nir_deref *deref)
> > +{
> > + while (deref->child != NULL)
> > + deref = deref->child;
> > + return deref;
> > +}
>
> I think long-term, we should probably make dereferences exec lists so
> we don't have to do things like this. In the meantime, though, I
> could've sworn I had to write this exact same function somewhere else
> (glsl to nir?)... might be a good idea to move this to nir.c and reuse
> it, unless nobody else needs this.
>
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.
And yes, I copied that function from lower_variables_scalar.
>
> > +
> > +static void
> > +nir_split_var_copy_instr(nir_intrinsic_instr *old_copy,
> > + nir_deref *dest_head, nir_deref *src_head,
> > + nir_deref *dest_tail, nir_deref *src_tail,
> > + struct split_var_copies_state *state)
>
> It took me a while to see exactly what this is doing, why we need
> separate dest_tail and src_tail variables even though they're almost
> always the same, and how it creates this temporary deref chain that's
> shared between source and destination (a no-no in NIR) but that's ok
> because they both get copied at the very end of the recursion when we
> actually emit the instructions. I'd like to see a comment explaining
> how this function works that explains those things.
>
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.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141217/f73b7d40/attachment.html>
More information about the mesa-dev
mailing list