[Mesa-dev] [PATCH] nir: switch fmul to increase chance of optimising it away

[Timothy Arceri]

On Thu, 2017-01-12 at 15:20 +1100, Timothy Arceri wrote:
> If one of the inputs to the multiplcation in ffma is the result of
> an fmul there is a chance that we can reuse the result of that
> fmul in other ffma calls if we do the multiplication in the right
> order.
> 
> For example it is a fairly common pattern for shaders to do something
> similar to this:
> 
>   const float a = 0.5;
>   in vec4 b;
>   in float c;
> 
>   ...
> 
>   b.x = b.x * c;
>   b.y = b.y * c;
> 
>   ...
> 
>   b.x = b.x * a + a;
>   b.y = b.y * a + a;
> 
> So by simply detecting that constant a is part of the multiplication
> in ffma and switching it with previous fmul that updates b we end up
> with:
> 
>   ...
> 
>   c = a * c;
> 
>   ...
> 
>   b.x = b.x * c + a;
>   b.y = b.y * c + a;
> 
> shader-db results BDW:
> 
> total instructions in shared programs: 13056473 -> 13038614 (-0.14%)
> instructions in affected programs: 2433641 -> 2415782 (-0.73%)
> helped: 10114
> HURT: 330
> 
> total cycles in shared programs: 256515402 -> 256331740 (-0.07%)
> cycles in affected programs: 137476650 -> 137292988 (-0.13%)
> helped: 10802
> HURT: 3871
> 
> total spills in shared programs: 14923 -> 14675 (-1.66%)
> spills in affected programs: 7976 -> 7728 (-3.11%)
> helped: 280
> HURT: 27
> 
> total fills in shared programs: 20141 -> 19691 (-2.23%)
> fills in affected programs: 11408 -> 10958 (-3.94%)
> helped: 282
> HURT: 27
> 
> LOST:   6
> GAINED: 1
> ---
>  src/compiler/nir/nir_opt_algebraic.py |  2 ++
>  src/compiler/nir/nir_search_helpers.h | 11 +++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/src/compiler/nir/nir_opt_algebraic.py
> b/src/compiler/nir/nir_opt_algebraic.py
> index b5974a7..3cddda3 100644
> --- a/src/compiler/nir/nir_opt_algebraic.py
> +++ b/src/compiler/nir/nir_opt_algebraic.py
> @@ -331,6 +331,8 @@ optimizations = [
>     (('~fadd', '#a', ('fadd', b, '#c')), ('fadd', ('fadd', a, c),
> b)),
>     (('iadd', '#a', ('iadd', b, '#c')), ('iadd', ('iadd', a, c), b)),
>  
> +   (('fadd', ('fmul(is_used_once)', ('fmul(is_used_once)',
> 'a(is_not_const)', 'b(is_not_const)'), '#c'), d), ('fadd', ('fmul',
> ('fmul', a, '#c'), b), d)),
> +
>     # Misc. lowering
>     (('fmod at 32', a, b), ('fsub', a, ('fmul', b, ('ffloor', ('fdiv',
> a, b)))), 'options->lower_fmod32'),
>     (('fmod at 64', a, b), ('fsub', a, ('fmul', b, ('ffloor', ('fdiv',
> a, b)))), 'options->lower_fmod64'),
> diff --git a/src/compiler/nir/nir_search_helpers.h
> b/src/compiler/nir/nir_search_helpers.h
> index ddaff52..f89e417 100644
> --- a/src/compiler/nir/nir_search_helpers.h
> +++ b/src/compiler/nir/nir_search_helpers.h
> @@ -115,6 +115,17 @@ is_zero_to_one(nir_alu_instr *instr, unsigned
> src, unsigned num_components,
>  }
>  
>  static inline bool
> +is_not_const(nir_alu_instr *instr, unsigned src, unsigned
> num_components)

I'm surprised this compiled and ran. There should be an extra param
here:

 const uint8_t *swizzle

Fixed locally.

> +{
> +   nir_const_value *val = nir_src_as_const_value(instr-
> >src[src].src);
> +
> +   if (val)
> +      return false;
> +
> +   return true;
> +}
> +
> +static inline bool
>  is_used_more_than_once(nir_alu_instr *instr)
>  {
>     bool zero_if_use = list_empty(&instr->dest.dest.ssa.if_uses);