[Mesa-dev] [PATCH 3/3] i965: Stop splitting fma() prior to optimization

[Jason Ekstrand]

On May 5, 2016 6:35 PM, "Connor Abbott" <cwabbott0 at gmail.com> wrote:
>
> On Thu, May 5, 2016 at 8:51 PM, Jason Ekstrand <jason at jlekstrand.net>
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 <kenneth at whitecape.org>
> > ---
> >  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
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160505/3574a545/attachment-0001.html>