[Mesa-dev] [PATCH v2 00/15] i965: Rework uniform handling in the back-end

[Jason Ekstrand]

On Thu, Apr 7, 2016 at 10:01 PM, Matt Turner <mattst88 at gmail.com> wrote:

> On Tue, Mar 22, 2016 at 3:33 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > This is mostly a re-send of a patch series I've had floating around in
> one
> > form or a while for quite some time.  It's basically the same except that
> > the original version was missing a work-around for Sandy Bridge.  For a
> > while, I wasn't really pushing to get it merged because I couldn't
> > demonstrate any actual performance benifit from pushing arrays.  However,
> > with the Vulkan API, the concept of push constants is directly exposed to
> > the user and we really need to be able to indirect on them.  This series
> > makes the FS backend 100% ready for indirect push constants;  vec4 will
> > take a little more work.
> >
> > It's worth noting that we've been carying these patches around in our
> > Vulkan driver for probably 3 or 4 months now and it's working great.
> >
> > For those that prefer to review on a branch:
> >
> > https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/i965-uniforms
> >
> > I think Kristian has mostly reviewed these patches.  However, he never
> sent
> > any R-Bs to the list.  I'd also like Ken or Matt to look at it from a
> > design perspective.
>
> I don't know what I think. I'm sympathetic to Curro's argument, but in
> the absence of more data it's hard to judge anything really. I'm not
> at all sympathetic to
>
> """
> Do I have a proof-of-concept in code, no.  However, I've run through
> it in my head and I have a pretty good idea what it would look like.
> You are free to go off and do it if you don't believe me, but I don't
> really want to hold things up while you do.
> """
>
> That's what... An Appeal to Your Brain? :)
>
> I don't know how to proceed on that front if no one is willing or
> interested in trying to implement it using reladdr.
>
> I ran shader-db.
>
> total instructions in shared programs: 7113290 -> 7161760 (0.68%)
> instructions in affected programs: 866011 -> 914481 (5.60%)
> helped: 0
> HURT: 7180
>
> total cycles in shared programs: 64705926 -> 64776118 (0.11%)
> cycles in affected programs: 4951554 -> 5021746 (1.42%)
> helped: 1605
> HURT: 5204
>
> of which the overwhelming majority is vertex shaders (why? this series
> is i965/fs). FS changes are just
>
> instructions in affected programs: 13550 -> 14132 (4.30%)
> helped: 0
> HURT: 50
>
> but I'm having a hard time finding shaders that actually use the
> address register.
>
> What's going on with the shader-db regressions?
>

As I figured, the shader-db issues are primarily a problem with
inconsistent use of D and UD.  This patch fixes that problem:

https://cgit.freedesktop.org/~jekstrand/mesa/commit/?h=wip/i965-uniforms-v3&id=0dbf9c8ee415b19073efe92fa586fddf22b725e6

I've pushed a version of the series with that patch to fd.o here:

https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/i965-uniforms-v3

There is still a small shader-db delta in vec4 which comes from changing
the algorithm we use to place pull constants in the buffer.  The original
algorithm walks over the instructions and places uniforms in the pull
constant buffer as it finds an instructions that use them.  The new
algorithm places them in the same order as their uniform number which is
effectively the order in which they are declared in the shader.

There are a number of shaders from Valve games (left4dead, portal 2, tf2)
that seem to have a single uniform array they use *a lot* and others that
they use less frequently.  The one they use most frequently happens to also
be used first so it gets placed first in the buffer with the old algorighm
and later with the new one.  The only reason this makes any difference
whatsoever is that whatever uniform gets placed first in the buffer is at
offset 0 so you don't need to add a constant offset to the array offset and
the address calculation has one less instruction.  In these Valve games,
that uniform happens to be used enough more often than the rest that that
extra instruction shows up in the shader-db results.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160408/b2a9a630/attachment.html>