Forwarding to the mailing list because I'm an idiot and am apparently not able to email. > 2016-11-13 10:39 GMT+01:00 Timothy Arceri <<a href="mailto:timothy.arceri@collabora.com">timothy.arceri@collabora.com</a>>: > > On Sun, 2016-11-13 at 02:29 +0100, Thomas Helland 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 > > > > This is a small amount of shaders I take it this is just the public > > shader-db? > > > > It's an old one I've been running on for ages, so it's not very complete. > Some old dota and tf2 shaders, plus some other games. > I was actually planning on scraping a reasonable collection of games > for shaders this evening, as I have the time. > > > While the series is interesting I think there are more robust ways to > > deal with the issue of slow compile times. Landing the shader-cache > > (once I rebase it on the recent changes I've made) is obviously one but > > we are also getting very close to being able to use the NIR > > optimisation passes from the GLSL IR linker (at least for things that > > have a NIR backend). Once we do that we can work towards only calling > > the GLSL IR passes from the compiler. > > > > Since I landed changes to switch i965 to using nir_gather_shader_info() > > this week the only big blocker for using NIR opts in the linker is a > > varying packing pass for NIR, once we have that is should be fairly > > easy to wire things up like so in brw_create_nir() for i965: > > > > nir = brw_preprocess_nir(brw->screen->compiler, nir); > > > > //TODO: remove from GLSL IR the varyings/uniform that the nir opt > > // passes removed. This is safe because at this point all we want > > // from the GLSL IR is to assign locations and do validation on > > // varyings and uniforms we don't care if we make the IR invalid. > > > > //TODO: call GLSL assign varying/uniform location/validation passes > > > > //TODO: now remove from nir any more varyings that GLSL IR removed > > // (this is safe because these will just be varyings that are only > > // used in one of the stages) and add locations that were assigned > > // in GLSL to NIR. > > > > //TODO: free GLSL IR > > > > //TODO: add a NIR varying packing pass here > > > > We also need theses patches I'm still waiting on review for [1][2]. > > > > Anyway the point is IMO your time would probably be better spent > > working towards improving deficiencies in the NIR opts so we can switch > > to only using them faster as opposed to spending to much time improving > > the GLSL IR opts. > > > > > > [1] <a href="https://patchwork.freedesktop.org/patch/112044/">https://patchwork.freedesktop.org/patch/112044/</a> > > [2] <a href="https://patchwork.freedesktop.org/patch/112045/">https://patchwork.freedesktop.org/patch/112045/</a> > > > > Yes, I definitely agree that this is just a band aid, and doesn't fix > the real issue. > It was merely a simple test to check what effects this could probably have, > while being not that invasive of a change. > > > > >> 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); > >>