[Mesa-dev] [PATCH 5/9] nir: Add a dedicated ffma peephole optimization

[Matt Turner]

On Mon, Mar 23, 2015 at 8:13 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> ---
>  src/glsl/Makefile.sources            |   1 +
>  src/glsl/nir/nir_opt_peephole_ffma.c | 220 +++++++++++++++++++++++++++++++++++
>  2 files changed, 221 insertions(+)
>  create mode 100644 src/glsl/nir/nir_opt_peephole_ffma.c
>
> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
> index b876642..1685a05 100644
> --- a/src/glsl/Makefile.sources
> +++ b/src/glsl/Makefile.sources
> @@ -47,6 +47,7 @@ NIR_FILES = \
>         nir/nir_opt_dce.c \
>         nir/nir_opt_gcm.c \
>         nir/nir_opt_global_to_local.c \
> +       nir/nir_opt_peephole_ffma.c \
>         nir/nir_opt_peephole_select.c \
>         nir/nir_opt_remove_phis.c \
>         nir/nir_print.c \
> diff --git a/src/glsl/nir/nir_opt_peephole_ffma.c b/src/glsl/nir/nir_opt_peephole_ffma.c
> new file mode 100644
> index 0000000..4674505
> --- /dev/null
> +++ b/src/glsl/nir/nir_opt_peephole_ffma.c
> @@ -0,0 +1,220 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Jason Ekstrand (jason at jlekstrand.net)
> + *
> + */
> +
> +#include "nir.h"
> +
> +/*
> + * Implements a small peephole optimization that looks for a multiply that
> + * is only ever used in an add and replaces both with an fma.
> + */
> +
> +struct peephole_ffma_state {
> +   void *mem_ctx;
> +   nir_function_impl *impl;
> +   bool progress;
> +};
> +
> +static inline nir_alu_instr *

Don't mark as inline. Let gcc make the decision unless you really know
something that it doesn't.

In this case, I don't think you do, since this function is recursive. :)

> +get_mul_for_src(nir_alu_src *src, uint8_t swizzle[4], bool *negate, bool *abs)
> +{
> +   assert(src->src.is_ssa && !src->abs && !src->negate);
> +
> +   nir_instr *instr = src->src.ssa->parent_instr;
> +   if (instr->type != nir_instr_type_alu)
> +      return NULL;
> +
> +   nir_alu_instr *alu = nir_instr_as_alu(instr);
> +   switch (alu->op) {
> +   case nir_op_imov:
> +   case nir_op_fmov:
> +      alu = get_mul_for_src(&alu->src[0], swizzle, negate, abs);
> +      break;
> +
> +   case nir_op_fneg:
> +      alu = get_mul_for_src(&alu->src[0], swizzle, negate, abs);
> +      *negate = !*negate;
> +      break;
> +
> +   case nir_op_fabs:
> +      alu = get_mul_for_src(&alu->src[0], swizzle, negate, abs);
> +      *negate = false;
> +      *abs = true;
> +      break;
> +
> +   case nir_op_fmul:
> +      break;
> +
> +   default:
> +      return NULL;
> +   }
> +
> +   if (!alu)
> +      return NULL;
> +
> +   for (unsigned i = 0; i < 4; i++) {
> +      if (!(alu->dest.write_mask & (1 << i)))
> +         break;
> +
> +      swizzle[i] = swizzle[src->swizzle[i]];
> +   }
> +
> +   return alu;
> +}
> +
> +static bool
> +nir_opt_peephole_ffma_block(nir_block *block, void *void_state)
> +{
> +   struct peephole_ffma_state *state = void_state;
> +
> +   nir_foreach_instr_safe(block, instr) {
> +      if (instr->type != nir_instr_type_alu)
> +         continue;
> +
> +      nir_alu_instr *add = nir_instr_as_alu(instr);
> +      if (add->op != nir_op_fadd)
> +         continue;
> +
> +      /* TODO: Maybe bail if this expression is considered "precise"? */
> +
> +      assert(add->src[0].src.is_ssa && add->src[1].src.is_ssa);
> +
> +      /* This, is the case a + a.  We would rather handle this with an
> +       * algebraic reduction than fuse it.  Also, we want to only fuse
> +       * things where the multiply is used only once and, in this case,
> +       * it would be used twice by the same instruction.
> +       */
> +      if (add->src[0].src.ssa == add->src[1].src.ssa)
> +         continue;
> +
> +      nir_alu_instr *mul;
> +      uint8_t add_mul_src, swizzle[4];
> +      bool negate, abs;
> +      for (add_mul_src = 0; add_mul_src < 2; add_mul_src++) {
> +         for (unsigned i = 0; i < 4; i++)
> +            swizzle[i] = i;
> +
> +         negate = false;
> +         abs = false;
> +
> +         mul = get_mul_for_src(&add->src[add_mul_src], swizzle, &negate, &abs);
> +
> +         if (mul != NULL)
> +            break;
> +      }
> +
> +      if (mul == NULL)
> +         continue;
> +
> +      nir_ssa_def *mul_src[2];
> +      mul_src[0]= mul->src[0].src.ssa;
> +      mul_src[1]= mul->src[1].src.ssa;

Space before =

With those trivial things fixed (and probably 5+6 squashed), this is

Reviewed-by: Matt Turner <mattst88 at gmail.com>