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

[Jason Ekstrand]

On Sep 15, 2016 4:31 PM, "Timothy Arceri" <timothy.arceri at collabora.com>
wrote:
>
> On Thu, 2016-09-15 at 12:34 -0700, Jason Ekstrand wrote:
> > On Sep 15, 2016 12:05 AM, "Timothy Arceri" <timothy.arceri at collabora.
> > 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.
> > >
> > > 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
> > > ---
> > >  src/intel/vulkan/anv_pipeline.c        | 10 ----------
> > >  src/mesa/drivers/dri/i965/brw_link.cpp | 26 ++++++++++++++------
> > ------
> > >  src/mesa/drivers/dri/i965/brw_nir.c    | 12 ++++++++++++
> > >  3 files changed, 26 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_pipeline.c
> > b/src/intel/vulkan/anv_pipeline.c
> > > index f96fe22..f292f0b 100644
> > > --- a/src/intel/vulkan/anv_pipeline.c
> > > +++ b/src/intel/vulkan/anv_pipeline.c
> > > @@ -183,16 +183,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 2b1fa61..41791d4 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> > > @@ -139,18 +139,20 @@ process_glsl_ir(gl_shader_stage stage,
> > >
> > >     do_copy_propagation(shader->ir);
> > >
> > > -   bool lowered_variable_indexing =
> > > -      lower_variable_index_to_cond_assign((gl_shader_stage)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));
> > > +   if (brw->gen < 7) {
> > > +      bool lowered_variable_indexing =
> > > +
> >  lower_variable_index_to_cond_assign((gl_shader_stage)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;
> > > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c
> > b/src/mesa/drivers/dri/i965/brw_nir.c
> > > index e8dafae..af646ed 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_nir.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> > > @@ -453,6 +453,18 @@ brw_preprocess_nir(const struct brw_compiler
> > *compiler, nir_shader *nir)
> > >     /* Lower a bunch of stuff */
> > >     OPT_V(nir_lower_var_copies);
> > >
> > > +   if (compiler->devinfo->gen > 6) {
> > I think you want "> 7" here
>
> It can be used with gen 7 and up. I could change it to >= 7 if that is
> easier to parse but I think > 6 is functionally correct.

Hunh? You use GLSL for gen7. Why are we duplicating? Also, why can't this
be used on sandy bridge and earlier?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160915/110621ef/attachment.html>