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>
This commit is contained in:
Alyssa Rosenzweig 2019-05-15 01:16:51 +00:00
parent 81b1053d9b
commit 0a13babdd8
2 changed files with 23 additions and 7 deletions

View file

@ -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;
}

View file

@ -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 */