On May 5, 2016 6:35 PM, "Connor Abbott" <<a href="mailto:cwabbott0@gmail.com">cwabbott0@gmail.com</a>> wrote: > > On Thu, May 5, 2016 at 8:51 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > > According to the GLSL spec, if the user uses fma directly and you have it > > in your hardware, you shouldn't split it. For a while now, we've been > > splitting all fma's up-front and then planned to fuse them later. The only > > reason why this possibly helped before was for ARB programs which is > > handled by the previous commit. This fixes rendering corruptions in Tomb > > Raider. > > Where in the GLSL spec does it say this? In the definition of fma(), it says: > > "Otherwise, in the absence of precise consumption, there are no > special constraints on the number of operations or difference in > precision between fma() and the expression > 'a * b + c'." > > So in other words, splitting them and then opportunistically combining > them is fine. I think the real problem is that the lower_ffma > transform isn't marked as inexact, when doing that transform does > violate the GLSL spec. That is, we shouldn't be splitting ffma > instructions marked as exact. Sure, but that isn't lowering. Drivers that need lowering want it split regardless. If we want to just split imprecise things then we need a new split_imprecise_ffma option. I should at least update the comment. > > > > Shader-db results on Haswell: > > > > total instructions in shared programs: 7560300 -> 7561510 (0.02%) > > instructions in affected programs: 56265 -> 57475 (2.15%) > > helped: 86 > > HURT: 291 > > > > The only shaders in the database that are affected are from Shadow of > > Mordor which is the first app in our database to use fma(). > > > > Reported-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> > > --- > > src/mesa/drivers/dri/i965/brw_compiler.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_compiler.c b/src/mesa/drivers/dri/i965/brw_compiler.c > > index 0ea5e8b..7969543 100644 > > --- a/src/mesa/drivers/dri/i965/brw_compiler.c > > +++ b/src/mesa/drivers/dri/i965/brw_compiler.c > > @@ -72,7 +72,6 @@ shader_perf_log_mesa(void *data, const char *fmt, ...) > > * split all ffma instructions during opt_algebraic and we then re-combine \ > > * them as a later step. \ > > */ \ > > - .lower_ffma = true, \ > > .lower_sub = true, \ > > .lower_fdiv = true, \ > > .lower_scmp = true, \ > > -- > > 2.5.0.400.gff86faf > > > > _______________________________________________ > > mesa-dev mailing list > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>