[Mesa-dev] [PATCH] nir: Allow to skip integer ops in nir_lower_to_source_mods

[Jason Ekstrand]

On November 11, 2018 07:44:54 Gert Wollny <gw.fossdev at gmail.com> wrote:

> From: Gert Wollny <gert.wollny at collabora.com>
>
>
> Since some hardware supports source mods only for float operations make
> it possible to skip this lowering for integer operations.

Out of curiosity, what hardware would that be?

> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
> I'm a bit unsure about what the best name for the parameter is, i.e. passing in
> true when one doesn't want all to be lowered seems a bit irritating, but a name
> like "lower_also_int_ops" looks ugly, and "lower_all" immediately asks for a
> comment what "not all" includes. I don't think it's worth adding option flags
> that would be more self-explanatory though.

Maybe a bit field:

typedef enum {
   nir_lower_int_source_mods = 1 << 0,
   nir_lower_float_source_mods = 1 << 1,
} nir_lower_to_source_mods_flags;

> thanks for any comments and reviews,
> Gert
>
>
>
>
> src/compiler/nir/nir.h                      |  2 +-
> src/compiler/nir/nir_lower_to_source_mods.c | 36 +++++++++++++--------
> src/intel/compiler/brw_nir.c                |  2 +-
> 3 files changed, 24 insertions(+), 16 deletions(-)
>
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index dc3c729dee..e2f64c9d44 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -3013,7 +3013,7 @@ typedef struct nir_lower_bitmap_options {
> void nir_lower_bitmap(nir_shader *shader, const nir_lower_bitmap_options 
> *options);
>
> bool nir_lower_atomics_to_ssbo(nir_shader *shader, unsigned ssbo_offset);
> -bool nir_lower_to_source_mods(nir_shader *shader);
> +bool nir_lower_to_source_mods(nir_shader *shader, bool skip_int_ops);
>
> bool nir_lower_gs_intrinsics(nir_shader *shader);
>
> diff --git a/src/compiler/nir/nir_lower_to_source_mods.c 
> b/src/compiler/nir/nir_lower_to_source_mods.c
> index 077ca53704..36f2160627 100644
> --- a/src/compiler/nir/nir_lower_to_source_mods.c
> +++ b/src/compiler/nir/nir_lower_to_source_mods.c
> @@ -34,7 +34,7 @@
>  */
>
> static bool
> -nir_lower_to_source_mods_block(nir_block *block)
> +nir_lower_to_source_mods_block(nir_block *block, bool skip_int_ops)
> {
>    bool progress = false;
>
> @@ -62,6 +62,8 @@ nir_lower_to_source_mods_block(nir_block *block)
>                continue;
>             break;
>          case nir_type_int:
> +            if (skip_int_ops)
> +               continue;
>             if (parent->op != nir_op_imov)
>                continue;
>             break;
> @@ -102,19 +104,10 @@ nir_lower_to_source_mods_block(nir_block *block)
>          alu->op = nir_op_fmov;
>          alu->dest.saturate = true;
>          break;
> -      case nir_op_ineg:
> -         alu->op = nir_op_imov;
> -         alu->src[0].negate = !alu->src[0].negate;
> -         break;
>       case nir_op_fneg:
>          alu->op = nir_op_fmov;
>          alu->src[0].negate = !alu->src[0].negate;
>          break;
> -      case nir_op_iabs:
> -         alu->op = nir_op_imov;
> -         alu->src[0].abs = true;
> -         alu->src[0].negate = false;
> -         break;
>       case nir_op_fabs:
>          alu->op = nir_op_fmov;
>          alu->src[0].abs = true;
> @@ -124,6 +117,21 @@ nir_lower_to_source_mods_block(nir_block *block)
>          break;
>       }
>
> +      if (!skip_int_ops) {
> +         switch (alu->op) {
> +         case nir_op_ineg:
> +            alu->op = nir_op_imov;
> +            alu->src[0].negate = !alu->src[0].negate;
> +            break;
> +         case nir_op_iabs:
> +            alu->op = nir_op_imov;
> +            alu->src[0].abs = true;
> +            alu->src[0].negate = false;
> +            break;
> +         default:
> +            break;
> +         }
> +      }
>       /* We've covered sources.  Now we're going to try and saturate the
>        * destination if we can.
>        */
> @@ -185,12 +193,12 @@ nir_lower_to_source_mods_block(nir_block *block)
> }
>
> static bool
> -nir_lower_to_source_mods_impl(nir_function_impl *impl)
> +nir_lower_to_source_mods_impl(nir_function_impl *impl, bool skip_int_ops)
> {
>    bool progress = false;
>
>    nir_foreach_block(block, impl) {
> -      progress |= nir_lower_to_source_mods_block(block);
> +      progress |= nir_lower_to_source_mods_block(block, skip_int_ops);
>    }
>
>    if (progress)
> @@ -201,13 +209,13 @@ nir_lower_to_source_mods_impl(nir_function_impl *impl)
> }
>
> bool
> -nir_lower_to_source_mods(nir_shader *shader)
> +nir_lower_to_source_mods(nir_shader *shader, bool skip_int_ops)
> {
>    bool progress = false;
>
>    nir_foreach_function(function, shader) {
>       if (function->impl) {
> -         progress |= nir_lower_to_source_mods_impl(function->impl);
> +         progress |= nir_lower_to_source_mods_impl(function->impl, 
> skip_int_ops);
>       }
>    }
>
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index cf5a4a96d6..9c5f2af030 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -793,7 +793,7 @@ brw_postprocess_nir(nir_shader *nir, const struct 
> brw_compiler *compiler,
>
>    OPT(nir_opt_algebraic_late);
>
> -   OPT(nir_lower_to_source_mods);
> +   OPT(nir_lower_to_source_mods, false);
>    OPT(nir_copy_prop);
>    OPT(nir_opt_dce);
>    OPT(nir_opt_move_comparisons);
> --
> 2.18.1