[Mesa-dev] [PATCH 2/2] glsl: Only call do_common_optimization once

[Thomas Helland]

On Nov 13, 2016 14:32, "Marek Olšák" <maraeo at gmail.com> wrote:
>
> Interesting idea. Also added this:
>

Oh, nice! Gotta catch them all :-)

> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index cc28b26..295d10b 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -4985,10 +4985,9 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
>           lower_tess_level(prog->_LinkedShaders[i]);
>        }
>
> -      while (do_common_optimization(prog->_LinkedShaders[i]->ir, true,
false,
> -                                    &ctx->Const.ShaderCompilerOptions[i],
> -                                    ctx->Const.NativeIntegers))
> -         ;
> +      do_common_optimization(prog->_LinkedShaders[i]->ir, true, false,
> +                            &ctx->Const.ShaderCompilerOptions[i],
> +                            ctx->Const.NativeIntegers);
>
>        lower_const_arrays_to_uniforms(prog->_LinkedShaders[i]->ir, i);
>        propagate_invariance(prog->_LinkedShaders[i]->ir);
> diff --git a/src/mesa/main/ff_fragment_shader.cpp
> b/src/mesa/main/ff_fragment_shader.cpp
> index 9aa1dc1..3890c54 100644
> --- a/src/mesa/main/ff_fragment_shader.cpp
> +++ b/src/mesa/main/ff_fragment_shader.cpp
> @@ -1251,9 +1251,9 @@ create_new_program(struct gl_context *ctx,
> struct state_key *key)
>     const struct gl_shader_compiler_options *options =
>        &ctx->Const.ShaderCompilerOptions[MESA_SHADER_FRAGMENT];
>
> -   while (do_common_optimization(p.shader->ir, false, false, options,
> -                                 ctx->Const.NativeIntegers))
> -      ;
> +   do_common_optimization(p.shader->ir, false, false, options,
> +                         ctx->Const.NativeIntegers);
> +
>     reparent_ir(p.shader->ir, p.shader->ir);
>
>     p.shader->CompileStatus = true;
> ------------------
>
> I've tested my private shader-db (26011 shaders, many games). I see
> only 2 regressions on radeonsi, both are changes in the code size:
>  WORST REGRESSIONS - Code Size
> Before     After     Delta Percentage
>  shaders/private/alien_isolation/788.shader_test [0]
>  8784      9080       296    3.37 %
>  shaders/private/unigine_tropics/264.shader_test [0]
>  1008      1012         4    0.40 %
>
> That's very good. Drivers not using LLVM might see worse regressions
though.
>
> Now, here are the compile time deltas.
> - GLSL source -> TGSI: -11.5% time
> - GLSL source -> GCN: -6% time
>

That's actually better than I had anticipated.
I hadn't gotten around to checking runtime impact.

> Overall, I like it.
>

Thanks for the feedback and testing :-)

> Marek
>
>
> On Sun, Nov 13, 2016 at 2:29 AM, Thomas Helland
> <thomashelland90 at gmail.com> wrote:
> > It turns out that only 8% of shaders benefit from more than one run
> > of the do_common_optimization function, now that it has the loop.
> > Doing only one run makes no difference to instruction count with NIR.
> >
> > 8324 shaders were run
> > 8128 benefit from 1 calls to do_common_optimization
> >  679 benefit from 2 calls to do_common_optimization
> >   69 benefit from 3 calls to do_common_optimization
> > ---
> >  src/compiler/glsl/glsl_parser_extras.cpp | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
b/src/compiler/glsl/glsl_parser_extras.cpp
> > index a5a0837..a1a1f90 100644
> > --- a/src/compiler/glsl/glsl_parser_extras.cpp
> > +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> > @@ -1944,9 +1944,8 @@ _mesa_glsl_compile_shader(struct gl_context *ctx,
struct gl_shader *shader,
> >        /* Do some optimization at compile time to reduce shader IR size
> >         * and reduce later work if the same shader is linked multiple
times
> >         */
> > -      while (do_common_optimization(shader->ir, false, false, options,
> > -                                    ctx->Const.NativeIntegers))
> > -         ;
> > +      do_common_optimization(shader->ir, false, false, options,
> > +                                    ctx->Const.NativeIntegers);
> >
> >        validate_ir_tree(shader->ir);
> >
> > --
> > 2.9.3
> >
> > _______________________________________________
> > 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/20161114/8340ef00/attachment-0001.html>