[Mesa-dev] [PATCH 02/10] i965: use nir_lower_indirect_derefs() for GLSL
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.
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 02/10] i965: use nir_lower_indirect_derefs() for GLSL
On Mon, Dec 5, 2016 at 5:12 PM, Timothy Arceri 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.
:-)
> 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@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
Re: [Mesa-dev] [PATCH 02/10] i965: use nir_lower_indirect_derefs() for GLSL
On Mon, 2016-12-12 at 01:20 -0800, Jason Ekstrand wrote:
> On Mon, Dec 5, 2016 at 5:12 PM, Timothy Arceri 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].EmitNoIndirectTem
