Re: [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.foss...@gmail.com> wrote:

From: Gert Wollny <gert.wol...@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.wol...@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

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev