[Mesa-dev] [PATCH 092/133] nir: Add a pass to lower local variables to registers

[Jason Ekstrand]

On Wed, Jan 14, 2015 at 1:10 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:

> On Fri, Jan 9, 2015 at 8:27 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> >
> > On Fri, Jan 9, 2015 at 4:38 PM, Connor Abbott <cwabbott0 at gmail.com>
> wrote:
> >>
> >> >>>> +      case nir_intrinsic_copy_var:
> >> >>>> +         unreachable("There should be no copies whatsoever at this
> >> >>>> point");
> >> >>>> +         break;
> >> >>>
> >> >>>
> >> >>> Are you sure about this? My impression is that lower_variables will
> >> >>> lower
> >> >>> copies involving things that aren't indirectly referenced, but if
> you
> >> >>> have
> >> >>> something like:
> >> >>>
> >> >>> foo[i] = ...
> >> >>> bar[*] = foo[*];
> >> >>> ... = bar[i];
> >> >>>
> >> >>> then the copy in the middle won't get lowered, unless there's
> >> >>> something
> >> >>> else I'm missing that will lower it.
> >> >>
> >> >>
> >> >> Yeah, there may be something missing there.  I have a pass lying
> around
> >> >> somewhere that lowers all copies.  Unfortunately, I've never actually
> >> >> seen
> >> >> this happen in the wild so It's untested.  I'll try and cook
> something
> >> >> up
> >> >> that I think is reliable.
> >> >
> >> >
> >> > Ok, more info.  Right now, GLSL IR is lowering all truely indirect
> >> > accesses
> >> > to if-ladders right now so we can never hit this.  Once we can handle
> >> > indirects in the backends or generate the if-ladders in NIR, we will
> >> > need
> >> > this.  Until then, let's leave it as-is to reduce the ammount of
> >> > untested
> >> > code.
> >> > --Jason
> >>
> >> Huh, that's weird... it lowers input/output indirect accesses to
> >> if-ladders, but not temporary indirect references, right? So wouldn't
> >> something like:
> >
> >
> > It appears to lower locals as well.
> >
> >>
> >>
> >> uniform int i;
> >> uniform bool f; /* false */
> >> in vec4 in_array[20];
> >> out vec4 color;
> >>
> >> void main()
> >> {
> >>    vec4 foo[20], bar[20], temp[20];
> >>    foo = in_array;
> >>    foo[i] = 0.0f;
> >>    while (f) {
> >>       temp = foo;
> >>       temp[i] = 1.0f;
> >>       foo = bar;
> >>       bar = temp;
> >>    }
> >>    bar[i] = 2.0f;
> >>    color = bar[0];
> >> }
> >>
> >> where GLSL IR can't eliminate the copies and doesn't turn the indirect
> >> references into if-ladders, and we can't lower them to SSA values,
> >> trigger the bug? If GLSL did turn everything into if-ladders, then
> >> this entire pass as well as half of lower_variables would be dead
> >> code...
> >
> >
> > I think we have a lot of dead code...
> >
>
> That's for sure! I thought about this a little more, and while you may
> not like it, I think we should still have the code to lower the
> remaining wildcard copies, even if it's dead code for now. Right now,
> I'd say more than half of the variable lowering code is currently
> dead, so I'm not so swayed by the "but it's more dead code!" argument
> -- we already have a *lot* of dead code, so let's at least fix a known
> bug in it rather than making it even more confusing. I think that the
> only way it would make sense to not go all the way is if we stripped
> out everything else, but that doesn't make that much sense after I
> went through all this work to review it all.
>

Agreed.  I'll put a pass together.  It shouldn't take me long.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150114/162f43f7/attachment-0001.html>