[Mesa-dev] [PATCH] nir/deref: Rematerialize parents in rematerialize_derefs_in_use_blocks

[apinheiro]

Reviewed-by: Alejandro Piñeiro <apinheiro at igalia.com>

On 11/2/19 5:28, Jason Ekstrand wrote:
> When nir_rematerialize_derefs_in_use_blocks_impl was first written, I
> attempted to optimize things a bit by not bothering to re-materialize
> the sources of deref instructions figuring that the final caller would
> take care of that.  However, in the case of more complex deref chains
> where the first link or two lives in block A and then another link and
> the load/store_deref intrinsic live in block B it doesn't work.  The
> code in rematerialize_deref_in_block looks at the tail of the chain,
> sees that it's already in block B and skips it, not realizing that part
> of the chain also lives in block A.
>
> The easy solution here is to just rematerialize deref sources of deref
> instructions as well.  This may potentially lead to a few more deref
> instructions being created by the conditions required for that to
> actually happen are fairly unlikely and, thanks to the caching, it's all
> linear time regardless.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109603
> ---
>   src/compiler/nir/nir_deref.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/nir/nir_deref.c b/src/compiler/nir/nir_deref.c
> index 13aa10c7532..7b56611915d 100644
> --- a/src/compiler/nir/nir_deref.c
> +++ b/src/compiler/nir/nir_deref.c
> @@ -574,10 +574,9 @@ nir_rematerialize_derefs_in_use_blocks_impl(nir_function_impl *impl)
>            _mesa_hash_table_clear(state.cache, NULL);
>   
>         nir_foreach_instr_safe(instr, block) {
> -         if (instr->type == nir_instr_type_deref) {
> -            nir_deref_instr_remove_if_unused(nir_instr_as_deref(instr));
> +         if (instr->type == nir_instr_type_deref &&
> +             nir_deref_instr_remove_if_unused(nir_instr_as_deref(instr)))
>               continue;
> -         }
>   
>            state.builder.cursor = nir_before_instr(instr);
>            nir_foreach_src(instr, rematerialize_deref_src, &state);