On Wed, Jan 14, 2015 at 1:10 PM, Connor Abbott <cwabbo...@gmail.com> wrote:
> On Fri, Jan 9, 2015 at 8:27 PM, Jason Ekstrand <ja...@jlekstrand.net>
> wrote:
> >
> >
> > On Fri, Jan 9, 2015 at 4:38 PM, Connor Abbott <cwabbo...@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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
