[Mesa-dev] [PATCH 8/8] i965: make use of nir linking

[Kenneth Graunke]

On Tuesday, September 12, 2017 4:37:35 PM PDT Timothy Arceri wrote:
> For now linking is just removing unused varyings between stages.
> 
> shader-db results BDW:
> 
> total instructions in shared programs: 13198288 -> 13191693 (-0.05%)
> instructions in affected programs: 48325 -> 41730 (-13.65%)
> helped: 473
> HURT: 0
> 
> total cycles in shared programs: 541184926 -> 541159260 (-0.00%)
> cycles in affected programs: 213238 -> 187572 (-12.04%)
> helped: 435
> HURT: 8
> ---
>  src/mesa/drivers/dri/i965/brw_link.cpp | 48 ++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
> index 9f1634a5459..c4c89a0686e 100644
> --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> @@ -252,6 +252,54 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
>                                   compiler->scalar_stage[stage]);
>     }
>  
> +   /* Determine first and last stage. */
> +   unsigned first = MESA_SHADER_STAGES;
> +   unsigned last = 0;
> +   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +      if (!shProg->_LinkedShaders[i])
> +         continue;
> +      if (first == MESA_SHADER_STAGES)
> +         first = i;
> +      last = i;
> +   }
> +
> +   /* Linking the stages in the opposite order (from fragment to vertex)
> +    * ensures that inter-shader outputs written to in an earlier stage
> +    * are eliminated if they are (transitively) not used in a later
> +    * stage.
> +    */
> +    if (first != last) {
> +       int next = last;
> +       for (int i = next - 1; i >= 0; i--) {
> +          if (shProg->_LinkedShaders[i] == NULL)
> +             continue;
> +
> +            nir_shader *producer = shProg->_LinkedShaders[i]->Program->nir;
> +            nir_shader *consumer = shProg->_LinkedShaders[next]->Program->nir;
> +
> +            nir_remove_dead_variables(shProg->_LinkedShaders[next]->Program->nir,
> +                                      nir_var_shader_in);
> +            nir_remove_dead_variables(shProg->_LinkedShaders[i]->Program->nir,
> +                                      nir_var_shader_out);
> +            if (nir_remove_unused_varyings(producer, consumer)) {
> +               nir_lower_global_vars_to_local(producer);
> +               nir_lower_global_vars_to_local(consumer);
> +
> +               nir_variable_mode indirect_mask = (nir_variable_mode) 0;
> +               if (compiler->glsl_compiler_options[i].EmitNoIndirectTemp)
> +                  indirect_mask = (nir_variable_mode) nir_var_local;
> +
> +               nir_lower_indirect_derefs(producer, indirect_mask);

Ugh, good catch...by demoting inputs/outputs to globals, we might end up
with indirects on temporaries, so we do need to lower them here.  (It's
a bit annoying that we don't handle indirects on temporaries in the FS
backend in 2017...I asked krh to implement that in review feedback for
his 2014-era SIMD8 VS series, and then jekstrand actually implemented it
in 2015, and it never landed for some reason...but...I digress...)

> +
> +               const bool is_scalar = compiler->scalar_stage[producer->stage];
> +               shProg->_LinkedShaders[i]->Program->nir =
> +                 brw_nir_optimize(producer, compiler, is_scalar);

I'm a little lost why you only lower indirect derefs and optimize for
the producer stage.  nir_remove_unused_varyings can demote variables in
either stage.  For example, if the consumer reads a variable, but the
producer doesn't write it, then we'll have a consumer-only variable get
demoted.

It seems like the consumer is the more important one, too, because
you're walking backwards, so the current producer may get hit by the
next pass...unless it's the first stage...so...don't we need to do both?

> +            }
> +
> +            next = i;
> +       }
> +    }
> +
>     for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
>        struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
>        if (!shader)
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170925/578acdbd/attachment.sig>