[Mesa-dev] [PATCH 3/5] nir: Do opt_algebraic in reverse order.

[Eduardo Lima Mitev]

On 02/05/2016 02:47 AM, Matt Turner wrote:
> Walking the SSA definitions in order means that we consider the smallest
> algebraic optimizations before larger optimizations. So if a smaller
> rule is part of a larger rule, the smaller one will happen first,
> preventing the larger one from happening.
> 
> instructions in affected programs: 32721 -> 32611 (-0.34%)
> helped: 106
> 
> Prevents regressions and annoyances in the next commits.
> ---
>  src/compiler/nir/nir_algebraic.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/compiler/nir/nir_algebraic.py b/src/compiler/nir/nir_algebraic.py
> index a30652f..77ad35e 100644
> --- a/src/compiler/nir/nir_algebraic.py
> +++ b/src/compiler/nir/nir_algebraic.py
> @@ -216,7 +216,7 @@ ${pass_name}_block(nir_block *block, void *void_state)
>  {
>     struct opt_state *state = void_state;
>  
> -   nir_foreach_instr_safe(block, instr) {
> +   nir_foreach_instr_reverse_safe(block, instr) {

I would add an explicit comment here as to why walk in reverse order. It
is not immediately clear (at least to me) that the smallest algebraic
optimizations come before the larger ones. I could not find any comment
in opt_algebraic.py or anywhere else that would suggest this is the case.

>        if (instr->type != nir_instr_type_alu)
>           continue;
>  
> @@ -255,7 +255,7 @@ ${pass_name}_impl(nir_function_impl *impl, const bool *condition_flags)
>     state.progress = false;
>     state.condition_flags = condition_flags;
>  
> -   nir_foreach_block(impl, ${pass_name}_block, &state);
> +   nir_foreach_block_reverse(impl, ${pass_name}_block, &state);
>

Does it make sense to reverse traversing of blocks too? As far as I
understand opt_algebraic rules don't expand to other blocks (maybe I'm
wrong). I also don't see any difference in shader-db results running
with or without this chunk.

These are my results on HSW (with patches 1 to 3):

total instructions in shared programs: 6265414 -> 6265312 (-0.00%)
instructions in affected programs: 31499 -> 31397 (-0.32%)
helped: 98
HURT: 0

total cycles in shared programs: 56081290 -> 56078442 (-0.01%)
cycles in affected programs: 562440 -> 559592 (-0.51%)
helped: 102
HURT: 6


Patches 1 to 3 are:

Reviewed-by: Eduardo Lima Mitev <elima at igalia.com>

>     if (state.progress)
>        nir_metadata_preserve(impl, nir_metadata_block_index |
>