On Sat, Jul 28, 2018 at 10:44:42PM -0700, Jason Ekstrand wrote:
> We have to be a bit careful with this one because we want it to run in
> the optimization loop but only in the first brw_nir_optimize call.
> Later calls assume that we've lowered away copy_deref instructions and
> we don't want to introduce any more.
>
> Shader-db results on Kaby Lake:
>
> total instructions in shared programs: 15176942 -> 15176942 (0.00%)
> instructions in affected programs: 0 -> 0
> helped: 0
> HURT: 0
>
> In spite of the lack of any shader-db improvement, this patch completely
> eliminates spilling in the Batman: Arkham City tessellation shaders.
> This is because we are now able to detect that the temporary array
> created by DXVK for storing TCS inputs is a copy of the input arrays and
> use indirect URB reads instead of making a copy of 4.5 KiB of input data
> and then indirecting on it with if-ladders.
> ---
> src/intel/compiler/brw_nir.c | 16 +++++++++-------
> src/intel/compiler/brw_nir.h | 3 ++-
> 2 files changed, 11 insertions(+), 8 deletions(-)
Given the comment mentioned below is added. This patch is
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.olive...@intel.com>
There's also an optional suggestion.
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index 5e9da9e1ef2..2417c0cd618 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -533,7 +533,7 @@ brw_nir_no_indirect_mask(const struct brw_compiler
> *compiler,
>
> nir_shader *
> brw_nir_optimize(nir_shader *nir, const struct brw_compiler *compiler,
> - bool is_scalar)
> + bool is_scalar, bool allow_copies)
> {
> nir_variable_mode indirect_mask =
> brw_nir_no_indirect_mask(compiler, nir->info.stage);
> @@ -544,6 +544,8 @@ brw_nir_optimize(nir_shader *nir, const struct
> brw_compiler *compiler,
> OPT(nir_split_array_vars, nir_var_local);
> OPT(nir_shrink_vec_array_vars, nir_var_local);
> OPT(nir_lower_vars_to_ssa);
> + if (allow_copies)
> + OPT(nir_opt_find_array_copies);
> OPT(nir_opt_copy_prop_vars);
>
> if (is_scalar) {
Adding a comment here with a subset of your first paragraph in the commit
message will make reduce the chance of future changes to optimize mess
things up.
> @@ -664,7 +666,7 @@ brw_preprocess_nir(const struct brw_compiler *compiler,
> nir_shader *nir)
> nir_lower_isign64 |
> nir_lower_divmod64);
>
> - nir = brw_nir_optimize(nir, compiler, is_scalar);
> + nir = brw_nir_optimize(nir, compiler, is_scalar, true);
>
> /* This needs to be run after the first optimization pass but before we
> * lower indirect derefs away
> @@ -701,7 +703,7 @@ brw_preprocess_nir(const struct brw_compiler *compiler,
> nir_shader *nir)
> nir_lower_indirect_derefs(nir, indirect_mask);
>
> /* Get rid of split copies */
> - nir = brw_nir_optimize(nir, compiler, is_scalar);
> + nir = brw_nir_optimize(nir, compiler, is_scalar, false);
Optional: the call-sites usually have a "is_scalar" variable around,
making that boolean parameter "self-documenting". This is not the
case for allow_copies. One idea is to make this last argument a flag,
and add an enum for the ALLOW_CREATE_DEREF_COPIES or something like.
Thanks,
Caio
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
