[Mesa-dev] [PATCH 1/3] panfrost/midgard: Lower source modifiers for ints

[Alyssa Rosenzweig]

On Midgard, float ops support standard source modifiers (abs/neg) and
destination modifiers (sat/pos/round). Integer ops do not support these,
however. To cope, we use native NIR source modifiers for floats, but
lower them away to iabs/ineg for integers, implementing those ops
simultaneously to avoid regressions.

Fixes the integer tests in
dEQP-GLES2.functional.shaders.operator.unary_operator.minus.*

Signed-off-by: Alyssa Rosenzweig <alyssa at rosenzweig.io>
---
 src/gallium/drivers/panfrost/midgard/helpers.h       |  2 ++
 .../drivers/panfrost/midgard/midgard_compile.c       |  9 +++++++--
 src/gallium/drivers/panfrost/midgard/midgard_nir.h   |  1 +
 .../panfrost/midgard/midgard_nir_algebraic.py        | 12 ++++++++++++
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/panfrost/midgard/helpers.h b/src/gallium/drivers/panfrost/midgard/helpers.h
index c90d7530ecb..01c88ed0fcf 100644
--- a/src/gallium/drivers/panfrost/midgard/helpers.h
+++ b/src/gallium/drivers/panfrost/midgard/helpers.h
@@ -142,6 +142,7 @@ midgard_is_integer_op(int op)
         //case midgard_alu_op_f2i:
         //case midgard_alu_op_f2u:
         case midgard_alu_op_ieq:
+        case midgard_alu_op_iabs:
         case midgard_alu_op_ine:
         case midgard_alu_op_ilt:
         case midgard_alu_op_ile:
@@ -209,6 +210,7 @@ static unsigned alu_opcode_props[256] = {
 
         /* Incredibly, iadd can run on vmul, etc */
         [midgard_alu_op_iadd]		 = UNITS_MOST,
+        [midgard_alu_op_iabs]		 = UNITS_MOST,
         [midgard_alu_op_isub]		 = UNITS_MOST,
         [midgard_alu_op_imul]		 = UNITS_MOST,
         [midgard_alu_op_imov]		 = UNITS_MOST | QUIRK_FLIPPED_R24,
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 5147e85fa26..dac5eeb2c1b 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -748,9 +748,13 @@ optimise_nir(nir_shader *nir)
         } while (progress);
 
         NIR_PASS(progress, nir, nir_opt_algebraic_late);
+        NIR_PASS(progress, nir, midgard_nir_lower_algebraic_late);
 
-        /* Lower mods */
-        NIR_PASS(progress, nir, nir_lower_to_source_mods, nir_lower_all_source_mods);
+        /* Lower mods for float ops only. Integer ops don't support modifiers
+         * (saturate doesn't make sense on integers, neg/abs require dedicated
+         * instructions) */
+
+        NIR_PASS(progress, nir, nir_lower_to_source_mods, nir_lower_float_source_mods);
         NIR_PASS(progress, nir, nir_copy_prop);
         NIR_PASS(progress, nir, nir_opt_dce);
 
@@ -975,6 +979,7 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
                 ALU_CASE(iadd, iadd);
                 ALU_CASE(isub, isub);
                 ALU_CASE(imul, imul);
+                ALU_CASE(iabs, iabs);
 
                 /* XXX: Use fmov, not imov, since imov was causing major
                  * issues with texture precision? XXX research */
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_nir.h b/src/gallium/drivers/panfrost/midgard/midgard_nir.h
index b7a22980502..6b4833b32b2 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_nir.h
+++ b/src/gallium/drivers/panfrost/midgard/midgard_nir.h
@@ -2,4 +2,5 @@
 #include "nir.h"
 
 bool midgard_nir_lower_algebraic(nir_shader *shader);
+bool midgard_nir_lower_algebraic_late(nir_shader *shader);
 bool midgard_nir_scale_trig(nir_shader *shader);
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_nir_algebraic.py b/src/gallium/drivers/panfrost/midgard/midgard_nir_algebraic.py
index 07ea427359b..05a49d18d9e 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_nir_algebraic.py
+++ b/src/gallium/drivers/panfrost/midgard/midgard_nir_algebraic.py
@@ -39,6 +39,15 @@ algebraic = [
     (('fsign', a), ('bcsel', ('fge', a, 0), 1.0, -1.0)),
 ]
 
+algebraic_late = [
+    # ineg must be lowered late, but only for integers; floats will try to
+    # have modifiers attached... hence why this has to be here rather than
+    # a more standard lower_negate approach
+
+    (('ineg', a), ('isub', 0, a)),
+]
+
+
 # Midgard scales fsin/fcos arguments by pi.
 # Pass must be run only once, after the main loop
 
@@ -62,6 +71,9 @@ def run():
     print(nir_algebraic.AlgebraicPass("midgard_nir_lower_algebraic",
                                       algebraic).render())
 
+    print(nir_algebraic.AlgebraicPass("midgard_nir_lower_algebraic_late",
+                                      algebraic_late).render())
+
     print(nir_algebraic.AlgebraicPass("midgard_nir_scale_trig",
                                       scale_trig).render())
 
-- 
2.20.1