panfrost/midgard: Set int outmod for ops writing integers

By default, the "normal" output modifier is set on ALU ops. This is the
correct default for float outputs -- for floats, it preserves the semantic
value. Unfortunately, when used with integers, it does not preserve the
bitstream encoding, causing misbehaviour. (It's an open question what
happens when `normal` is used with integers -- does it apply some other
transformation? or does it do floating point normalization/etc on the
ints as if they were floats?).

Instead, we default to the "clamp to integer" output modifier for
ops writing integers. Semantically, this makes sense (clamping an
integer to the nearest integer is the identity function). In the
hardware with an integer opcode, this is the actual "normal".

This fixes numerous sporadic and sometimes bizarre bugs relating to
integers, especially integer moves. With this in place, we no longer
care about the types involved; it's just bits on the wire again.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>

diff --git a/src/gallium/drivers/panfrost/midgard/helpers.h b/src/gallium/drivers/panfrost/midgard/helpers.h
index dc2de15931bc842c3e5ee549bec0813d819cdac5..441c72858875ddcd526057b530c58b83f3c5136c 100644 (file)
--- a/src/gallium/drivers/panfrost/midgard/helpers.h
+++ b/src/gallium/drivers/panfrost/midgard/helpers.h
@@ -71,6 +71,9 @@
 /* Is the op commutative? */
 #define OP_COMMUTES (1 << 3)
 
+/* Does the op convert types between int- and float- space (i2f/f2u/etc) */
+#define OP_TYPE_CONVERT (1 << 4)
+
 /* Vector-independant shorthands for the above; these numbers are arbitrary and
  * not from the ISA. Convert to the above with unit_enum_to_midgard */
 
@@ -207,11 +210,11 @@ static struct {
         [midgard_alu_op_fexp2]          = {"fexp2", UNIT_VLUT},
         [midgard_alu_op_flog2]          = {"flog2", UNIT_VLUT},
 
-        [midgard_alu_op_f2i]            = {"f2i", UNITS_ADD},
-        [midgard_alu_op_f2u]            = {"f2u", UNITS_ADD},
-        [midgard_alu_op_f2u8]           = {"f2u8", UNITS_ADD},
-        [midgard_alu_op_i2f]            = {"i2f", UNITS_ADD},
-        [midgard_alu_op_u2f]            = {"u2f", UNITS_ADD},
+        [midgard_alu_op_f2i]            = {"f2i", UNITS_ADD | OP_TYPE_CONVERT},
+        [midgard_alu_op_f2u]            = {"f2u", UNITS_ADD | OP_TYPE_CONVERT},
+        [midgard_alu_op_f2u8]           = {"f2u8", UNITS_ADD | OP_TYPE_CONVERT},
+        [midgard_alu_op_i2f]            = {"i2f", UNITS_ADD | OP_TYPE_CONVERT},
+        [midgard_alu_op_u2f]            = {"u2f", UNITS_ADD | OP_TYPE_CONVERT},
 
         [midgard_alu_op_fsin]           = {"fsin", UNIT_VLUT},
         [midgard_alu_op_fcos]           = {"fcos", UNIT_VLUT},
@@ -262,7 +265,7 @@ static struct {
 /* Is this opcode that of an integer (regardless of signedness)? Instruction
  * names authoritatively determine types */
 
-static bool
+static inline bool
 midgard_is_integer_op(int op)
 {
         const char *name = alu_opcode_props[op].name;
@@ -272,3 +275,15 @@ midgard_is_integer_op(int op)
 
         return (name[0] == 'i') || (name[0] == 'u');
 }
+
+/* Does this opcode *write* an integer? Same as is_integer_op, unless it's a
+ * conversion between int<->float in which case we do the opposite */
+
+static inline bool
+midgard_is_integer_out_op(int op)
+{
+        bool is_int = midgard_is_integer_op(op);
+        bool is_conversion = alu_opcode_props[op].props & OP_TYPE_CONVERT;
+
+        return is_int ^ is_conversion;
+}

diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 4a26ba769b2073c8f1b2ed12d630b0b18d08b88c..496ecb02e0968354b0de108b792a9ff0af192fae 100644 (file)
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -1336,8 +1336,9 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
                 return;
         }
 
-        /* Midgard can perform certain modifiers on output ofa n ALU op */
+        /* Midgard can perform certain modifiers on output of an ALU op */
         midgard_outmod outmod =
+                midgard_is_integer_out_op(op) ? midgard_outmod_int :
                 instr->dest.saturate ? midgard_outmod_sat : midgard_outmod_none;
 
         /* fmax(a, 0.0) can turn into a .pos modifier as an optimization */