[Mesa-dev] [PATCH] nir: Copy-propagate vecN operations that are actually moves

Jason Ekstrand jason at jlekstrand.net
Fri Feb 20 10:02:35 PST 2015


On Feb 20, 2015 9:27 AM, "Matt Turner" <mattst88 at gmail.com> wrote:
>
> On Fri, Feb 20, 2015 at 9:23 AM, Jason Ekstrand <jason at jlekstrand.net>
wrote:
> >
> >
> > On Thu, Feb 19, 2015 at 11:03 PM, Matt Turner <mattst88 at gmail.com>
wrote:
> >>
> >> On Thu, Feb 19, 2015 at 11:01 PM, Connor Abbott <cwabbott0 at gmail.com>
> >> wrote:
> >> > I agree with Ken that the regressions are small enough, and it seems
> >> > they're mostly stuff we can prevent by being smarter when doing the
> >> > sel peephole, so it seems like the cleanup that will probably help
> >> > other passes is worth it.
> >>
> >> So, usually we do that as a preparatory patch. Why aren't we doing that
> >> here?
> >
> >
> > Do what in a preparatory patch?  Fix up the sel peephole to be able to
> > handle "if (foo) bar = baz;"?  Sure, I can put that patch together.
>
> I thought that this would get turned into a conditional select as a
> side-effect of going out of SSA anyway (assuming baz was
> unconditionally set before the if)? Are these shaders not setting baz
> unconditionally?

The NIR peephole only triggers if the only instructions in the if or else
are moves.  In this particular case, it was a uniform load.  Before the
change, it was lowered to moves in both sides of the if and, since it ended
up being a push constant, got peepholed.  After the change, it was lowered
to moves on only one side of the if (which is theoretically better) and the
select didn't trigger.

> >>
> >> NIR instruction counts is not the metric we care about.
> >
> >
> > No, but cleaning things up means that we can do other optimizations
better.
> > Also, in each of those cases, the non-ssa NIR code was better it was
just
> > less cleanable by the backend.  We need to work on that, but I don't
think
> > it's an indicator of a problem.
> > --Jason
>
> Okay, that seems fine. I wouldn't bother updating the SEL peephole. It
> sounds like a number of the regressions we have are related to it not
> handling something so we might need to do a larger evaluation.

Yeah, it just needs to handle one-sided it's better.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150220/7ec586b4/attachment.html>


More information about the mesa-dev mailing list