summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlyssa Rosenzweig <[email protected]>2019-05-15 01:16:51 +0000
committerAlyssa Rosenzweig <[email protected]>2019-05-16 01:20:30 +0000
commit0a13babdd85ddfdca5ace32f0d79441ddb4dc21b (patch)
tree74644660fafdcba164497849151b4264de3f9362
parent81b1053d9b4f6e39a10ba3db79b4225172a23187 (diff)
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 <[email protected]>
-rw-r--r--src/gallium/drivers/panfrost/midgard/helpers.h27
-rw-r--r--src/gallium/drivers/panfrost/midgard/midgard_compile.c3
2 files changed, 23 insertions, 7 deletions
diff --git a/src/gallium/drivers/panfrost/midgard/helpers.h b/src/gallium/drivers/panfrost/midgard/helpers.h
index dc2de15931b..441c7285887 100644
--- 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 4a26ba769b2..496ecb02e09 100644
--- 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 */