On Thu, 2016-09-15 at 17:55 -0700, Jason Ekstrand wrote:
>
> > On Sep 15, 2016 4:31 PM, "Timothy Arceri" <timothy.arceri@collabora.c
om> wrote:
>
> >
>
> > On Thu, 2016-09-15 at 12:34 -0700, Jason Ekstrand wrote:
>
> > > > On Sep 15, 2016 12:05 AM, "Timothy Arceri" <timothy.arceri@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.
>
> > > >
>
> > > > 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?
GLSL IR is used for the non scalar stages on gen7. See my answers to
Connors questions:
https://lists.freedesktop.org/archives/mesa-dev/2016-September/129108.h
tml
> > > _______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
