From 0a13babdd85ddfdca5ace32f0d79441ddb4dc21b Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 15 May 2019 01:16:51 +0000 Subject: [PATCH] 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 --- .../drivers/panfrost/midgard/helpers.h | 27 ++++++++++++++----- .../panfrost/midgard/midgard_compile.c | 3 ++- 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 */ -- 2.30.2