[Mesa-dev] [PATCH 02/10] i965: use nir_lower_indirect_derefs() for GLSL

[Timothy Arceri]

On Mon, 2016-12-12 at 01:20 -0800, Jason Ekstrand wrote:
> On Mon, Dec 5, 2016 at 5:12 PM, Timothy Arceri <timothy.arceri at collab
> ora.com> wrote:
> > This moves the nir_lower_indirect_derefs() call into
> > brw_preprocess_nir() so thats is called by both OpenGL and Vulkan
> > and removes that call to the old GLSL IR pass
> > lower_variable_index_to_cond_assign()
> > 
> > We want to do this pass in nir to be able to move loop unrolling
> > to nir.
> > 
> > There is a increase of 1-3 instructions in a small number of
> > shaders,
> > and 2 Kerbal Space program shaders that increase by 32
> > instructions.
> > The changes seem to be caused be the difference in the GLSL IR vs
> > NIR variable index lowering passes. The GLSL IR pass creates a
> > simple if ladder, while the NIR pass implements a binary search.
> 
> Fun fact.  This patch helps the Gl43CSDof synmark test by about 12%. 
> Aparently, when you have an array of 40 things, the binary search
> matters. :-)

Cool. Hopefully that translates to improvements in games like bioshock-
infinite, tome raider, deus-ex, and metro-2033-reux all of which gain a
number of instructions in a number of shaders, although maybe not
enough to make a difference.

>  
> > Shader-db results BDW:
> > 
> > total instructions in shared programs: 8705873 -> 8706194 (0.00%)
> > instructions in affected programs: 32515 -> 32836 (0.99%)
> > helped: 3
> > HURT: 79
> > 
> > total cycles in shared programs: 74618120 -> 74583476 (-0.05%)
> > cycles in affected programs: 528104 -> 493460 (-6.56%)
> > helped: 47
> > HURT: 37
> > 
> > LOST:   2
> > GAINED: 0
> > 
> > V2: remove the do_copy_propagation() call from the i965 GLSL IR
> > linking code. This call was added in f7741c52111 but since we are
> > moving the variable index lowering to NIR we no longer need it and
> > can just rely on the nir copy propagation pass.
> > ---
> >  src/intel/vulkan/anv_pipeline.c        | 10 ----------
> >  src/mesa/drivers/dri/i965/brw_link.cpp | 15 ---------------
> >  src/mesa/drivers/dri/i965/brw_nir.c    | 10 ++++++++++
> >  3 files changed, 10 insertions(+), 25 deletions(-)
> > 
> > diff --git a/src/intel/vulkan/anv_pipeline.c
> > b/src/intel/vulkan/anv_pipeline.c
> > index 9b65e35..6b0a3c9 100644
> > --- a/src/intel/vulkan/anv_pipeline.c
> > +++ b/src/intel/vulkan/anv_pipeline.c
> > @@ -177,16 +177,6 @@ anv_shader_compile_to_nir(struct anv_device
> > *device,
> > 
> >     nir_shader_gather_info(nir, entry_point->impl);
> > 
> > -   nir_variable_mode indirect_mask = 0;
> > -   if (compiler->glsl_compiler_options[stage].EmitNoIndirectInput)
> > -      indirect_mask |= nir_var_shader_in;
> > -   if (compiler-
> > >glsl_compiler_options[stage].EmitNoIndirectOutput)
> > -      indirect_mask |= nir_var_shader_out;
> > -   if (compiler->glsl_compiler_options[stage].EmitNoIndirectTemp)
> > -      indirect_mask |= nir_var_local;
> > -
> > -   nir_lower_indirect_derefs(nir, indirect_mask);
> > -
> >     return nir;
> >  }
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp
> > b/src/mesa/drivers/dri/i965/brw_link.cpp
> > index 3f6041b..7902133 100644
> > --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> > @@ -135,21 +135,6 @@ process_glsl_ir(struct brw_context *brw,
> >     lower_noise(shader->ir);
> >     lower_quadop_vector(shader->ir, false);
> > 
> > -   do_copy_propagation(shader->ir);
> > -
> > -   bool lowered_variable_indexing =
> > -      lower_variable_index_to_cond_assign(shader->Stage, shader-
> > >ir,
> > -                                          options-
> > >EmitNoIndirectInput,
> > -                                          options-
> > >EmitNoIndirectOutput,
> > -                                          options-
> > >EmitNoIndirectTemp,
> > -                                          options-
> > >EmitNoIndirectUniform);
> > -
> > -   if (unlikely(brw->perf_debug && lowered_variable_indexing)) {
> > -      perf_debug("Unsupported form of variable indexing in %s;
> > falling "
> > -                 "back to very inefficient code generation\n",
> > -                 _mesa_shader_stage_to_abbrev(shader->Stage));
> > -   }
> > -
> >     bool progress;
> >     do {
> >        progress = false;
> > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c
> > b/src/mesa/drivers/dri/i965/brw_nir.c
> > index 763e3ec..8768cee 100644
> > --- a/src/mesa/drivers/dri/i965/brw_nir.c
> > +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> > @@ -485,6 +485,16 @@ brw_preprocess_nir(const struct brw_compiler
> > *compiler, nir_shader *nir)
> >     /* Lower a bunch of stuff */
> >     OPT_V(nir_lower_var_copies);
> > 
> > +   nir_variable_mode indirect_mask = 0;
> > +   if (compiler->glsl_compiler_options[nir-
> > >stage].EmitNoIndirectInput)
> > +      indirect_mask |= nir_var_shader_in;
> > +   if (compiler->glsl_compiler_options[nir-
> > >stage].EmitNoIndirectOutput)
> > +      indirect_mask |= nir_var_shader_out;
> > +   if (compiler->glsl_compiler_options[nir-
> > >stage].EmitNoIndirectTemp)
> > +      indirect_mask |= nir_var_local;
> > +
> > +   nir_lower_indirect_derefs(nir, indirect_mask);
> > +
> >     /* Get rid of split copies */
> >     nir = nir_optimize(nir, is_scalar);
> > 
> > --
> > 2.7.4
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev