[Mesa-dev] [PATCH 1/2] nir: add new nir_remove_dead_barriers() pass

[Bas Nieuwenhuizen]

On Wed, Nov 14, 2018 at 6:30 AM Timothy Arceri <tarceri at itsqueeze.com> wrote:
>
> Link time opts can remove unused outputs leaving behind useless
> barriers. This pass is intended to be called after linking so
> I've added it to nir_linking_helpers.c
>
> This pass removes some barriers from Witcher 3 (DXVK) Vulkan
> shaders.
> ---
>  src/compiler/nir/nir.h                 |  1 +
>  src/compiler/nir/nir_linking_helpers.c | 38 ++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index c469e111b2..224d2f66e2 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2817,6 +2817,7 @@ void nir_compact_varyings(nir_shader *producer, nir_shader *consumer,
>                            bool default_to_smooth_interp);
>  void nir_link_xfb_varyings(nir_shader *producer, nir_shader *consumer);
>  bool nir_link_constant_varyings(nir_shader *producer, nir_shader *consumer);
> +void nir_remove_dead_barriers(nir_shader *shader);
>
>  typedef enum {
>     /* If set, this forces all non-flat fragment shader inputs to be
> diff --git a/src/compiler/nir/nir_linking_helpers.c b/src/compiler/nir/nir_linking_helpers.c
> index a05890ada4..61982659a0 100644
> --- a/src/compiler/nir/nir_linking_helpers.c
> +++ b/src/compiler/nir/nir_linking_helpers.c
> @@ -669,3 +669,41 @@ nir_link_constant_varyings(nir_shader *producer, nir_shader *consumer)
>
>     return progress;
>  }
> +/* This is intended to be called this after linking opts as unused tcs outputs
> + * may have been removed leaving useless barriers in the shader.
> + *
> + * TODO: We could make this pass more complex and actually check if we are
> + * reading outputs set by other invocations.
> + */
> +void
> +nir_remove_dead_barriers(nir_shader *shader)
> +{
> +   if (shader->info.stage != MESA_SHADER_TESS_CTRL)
> +      return;
> +
> +   bool progress = false;
> +
> +   nir_function_impl *impl = nir_shader_get_entrypoint(shader);
> +
> +   nir_foreach_block(block, impl) {
> +      nir_foreach_instr_safe(instr, block) {
> +         if (instr->type != nir_instr_type_intrinsic)
> +            continue;
> +
> +         nir_intrinsic_instr *intr = nir_instr_as_intrinsic(instr);
> +
> +         /* If we see a store or load of an output then bail */
> +         if (intr->intrinsic == nir_intrinsic_store_deref ||
> +             intr->intrinsic == nir_intrinsic_load_deref) {
> +            nir_variable *var =
> +               nir_deref_instr_get_variable(nir_src_as_deref(intr->src[0]));
> +            if (var->data.mode == nir_var_shader_out)
> +               return;
> +         }
> +
> +         /* Remove barriers if we have not yet touched any outputs */
> +         if (intr->intrinsic == nir_intrinsic_barrier)
> +            nir_instr_remove(instr);
> +      }
> +   }

So we are going through blocks first to last right. Does this work
with loops? e.g. if we have something like

{
  for (...) {
    barrier();
    touch outputs
  }
}

In this case I'm not sure it is safe to remove the barrier but we might do so?

> +}
> --
> 2.19.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev