On Feb 12, 2015 1:28 AM, "Kenneth Graunke" <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote: > > On Wednesday, February 11, 2015 04:14:20 PM Ian Romanick wrote: > > On 02/11/2015 04:05 PM, Matt Turner wrote: > > > On Wed, Feb 11, 2015 at 3:51 PM, Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote: > > >> On 02/11/2015 02:54 PM, Matt Turner wrote: > > >>> We propagate negations to the right-most leaves of the multiplication > > >>> expression trees: > > >>> > > >>> - mul(neg(x), neg(y)) -> mul(x, y) > > >>> - mul(neg(x), y) -> neg(mul(x, y)) > > >>> - mul(x, neg(y)) -> neg(mul(x, y)) > > >>> > > >>> total instructions in shared programs: 5943123 -> 5937229 (-0.10%) > > >>> instructions in affected programs: 868221 -> 862327 (-0.68%) > > >>> helped: 4518 > > >>> HURT: 356 > > >>> GAINED: 1 > > >> > > >> This seems very counter intuitive. Why does this help? It seems like > > >> the negate modifier on the multiplication sources should be free... how > > >> do we get cheaper than free? :) > > > > > > Lots of shaders do things like var * 0.5 and var * -0.5. By making the > > > second of those -(var * 0.5) we can do the multiplication once and use > > > its result in both places, negated once. > > > > > > I also noticed we were generating a lot more LRPs after this. I guess > > > the LRP peephole is able to recognize more stuff without some negates > > > in the way. > > > > > > If you want, I can try to take a closer look at some affected shaders > > > -- though it is kind of hard. Diff'ing GLSL IR is impossible, and even > > > if you get past the compiler_temp@ variables being renumbered it's > > > still really difficult to see what's going on. > > > > Naw, that's fine. Enabling additional CSE is a perfectly valid > > explanation... though it makes me wonder if CSE should be smarter. For > > all the programs helped, about 8% are hurt. *shrug* > > CSE at the GLSL IR level is currently awful. It only handles variables > with the read_only flag set (to avoid dealing with kill sets). It also > starts at the leaves and works its way up - so if you have a huge > expression tree that's identical, it will CSE the tiniest piece, > introducing a new variable *without* read_only set, then completely bail > on the whole endeavor. > > I wasn't sure exactly what the semantics of read_only were, or if I > could just go set it on a new temporary. It's mostly used for uniforms, > IIRC. > > It's also not really worth fixing. SSA eliminates the need to track > variable kills, so NIR's pass doesn't have these restrictions and > already just works. We should just focus on moving to NIR. I'm OK with continuing to do stuff in the old IR as long as it's not too troublesome. However, it would be good if it also came with the corresponding 4-line NIR patch so we don't get even further out-of-sync. --Jason > _______________________________________________ > mesa-dev mailing list ><a href="mailto:mesa-dev@lists.freedesktop.org"> mesa-dev@lists.freedesktop.org</a> ><a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev"> http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> >