[Mesa-dev] [PATCH 03/13] i965: use nir_lower_indirect_derefs() for GLSL

[Jason Ekstrand]

On Wed, Dec 21, 2016 at 6:26 PM, 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.
> 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 for arrays of size 4 or less, while the NIR pass
> implements a binary search for all arrays regardless of size.
>
> Shader-db results BDW:
>
> total instructions in shared programs: 13021176 -> 13021819 (0.00%)
> instructions in affected programs: 57693 -> 58336 (1.11%)
> helped: 20
> HURT: 190
>
> total cycles in shared programs: 299805580 -> 299750826 (-0.02%)
> cycles in affected programs: 2290024 -> 2235270 (-2.39%)
> helped: 337
> HURT: 442
>
> total fills in shared programs: 19984 -> 19984 (0.00%)
> fills in affected programs: 0 -> 0
> helped: 0
> HURT: 0
>
> LOST:   4
> 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.
>
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  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 9104267..e2fbcab 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -190,16 +190,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 6f37428..0d8a626 100644
> --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> @@ -134,21 +134,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 55b16cf..7624126 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.c
> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> @@ -486,6 +486,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);
>

This needs to happen *after* lower_clip_cull_distance_arrays otherwise the
"compact" value won't get set on the clip and cull variables and
nir_lower_io will throw a fit.  Probably best to just move
lower_clip_cull_distance_arrays up.  With that change,

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> +
>     /* Get rid of split copies */
>     nir = nir_optimize(nir, is_scalar);
>
> --
> 2.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161222/fd85371b/attachment.html>