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