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

[Jason Ekstrand]

On November 11, 2018 09:11:26 Gert Wollny <gert.wol...@collabora.com> wrote:
Am Sonntag, den 11.11.2018, 09:07 -0600 schrieb 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?
r600/Evergreen at least.


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;

That was my first idea, but then it seemed a bit too much, i.e. I don't
know whether there is hardware that can do has source modes for int,
but not for float.

Meh. It makes it clear and that's the point. I don't really care if it's 
"too much" as long as the code remains readable.

As an aside, Intel hardware has source mods for integers but not all 
instructions support them and on Broadwell and later, logical instructions 
(and, or, xor) treat the negation modifier as doing an inot so it doesn't 
map perfectly for us either.


Best,
Gert


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



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