[Mesa-dev] [PATCH 3/6] panfrost/midgard: Add integer outmods
Alyssa Rosenzweig
alyssa.rosenzweig at collabora.com
Wed Jun 5 22:46:50 UTC 2019
For floats, output modifiers determine clamping behaviour. For integers,
they determine wrapping/saturation behaviour (or shifting -- see next
commit). These are very different; they are conceptually two unrelated
enums union'ed together; the distinction is responsible for many-a-bug.
While clamping behaviour for floats was clear from GL, the int behaviour
is only known From OpenCL contortion with convert_*_sat() functions.
With the underlying functions known, clean up the codebase, likely
fixing outmod type related bugs in the process.
Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig at collabora.com>
---
.../drivers/panfrost/midgard/disassemble.c | 22 ++++++---
.../drivers/panfrost/midgard/midgard.h | 17 +++++--
.../panfrost/midgard/midgard_compile.c | 46 +++++++++++++------
3 files changed, 60 insertions(+), 25 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/disassemble.c b/src/gallium/drivers/panfrost/midgard/disassemble.c
index c467e94fc29..1a25d0565f4 100644
--- a/src/gallium/drivers/panfrost/midgard/disassemble.c
+++ b/src/gallium/drivers/panfrost/midgard/disassemble.c
@@ -107,17 +107,25 @@ print_reg(unsigned reg, unsigned bits)
printf("r%u", reg);
}
-static char *outmod_names[4] = {
+static char *outmod_names_float[4] = {
"",
".pos",
- ".int",
+ ".unk2",
".sat"
};
+static char *outmod_names_int[4] = {
+ ".isat",
+ ".usat",
+ "",
+ ".unk3"
+};
+
static void
-print_outmod(midgard_outmod outmod)
+print_outmod(unsigned outmod, bool is_int)
{
- printf("%s", outmod_names[outmod]);
+ printf("%s", is_int ? outmod_names_int[outmod] :
+ outmod_names_float[outmod]);
}
static void
@@ -352,7 +360,8 @@ print_vector_field(const char *name, uint16_t *words, uint16_t reg_word,
printf("%s.", name);
print_alu_opcode(alu_field->op);
- print_outmod(alu_field->outmod);
+ print_outmod(alu_field->outmod,
+ midgard_is_integer_out_op(alu_field->op));
printf(" ");
bool out_high = false;
@@ -478,7 +487,8 @@ print_scalar_field(const char *name, uint16_t *words, uint16_t reg_word,
printf("%s.", name);
print_alu_opcode(alu_field->op);
- print_outmod(alu_field->outmod);
+ print_outmod(alu_field->outmod,
+ midgard_is_integer_out_op(alu_field->op));
printf(" ");
if (alu_field->output_full)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard.h b/src/gallium/drivers/panfrost/midgard/midgard.h
index c25f10c4028..b4eb788f813 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard.h
+++ b/src/gallium/drivers/panfrost/midgard/midgard.h
@@ -187,15 +187,22 @@ typedef enum {
typedef enum {
midgard_outmod_none = 0,
midgard_outmod_pos = 1,
- midgard_outmod_int = 2,
+ /* 0x2 unknown */
midgard_outmod_sat = 3
-} midgard_outmod;
+} midgard_outmod_float;
+
+typedef enum {
+ midgard_outmod_int_saturate = 0,
+ midgard_outmod_uint_saturate = 1,
+ midgard_outmod_int_wrap = 2,
+ /* 0x3 unknown */
+} midgard_outmod_int;
typedef enum {
midgard_reg_mode_8 = 0,
midgard_reg_mode_16 = 1,
midgard_reg_mode_32 = 2,
- midgard_reg_mode_64 = 3 /* TODO: verify */
+ midgard_reg_mode_64 = 3
} midgard_reg_mode;
typedef enum {
@@ -239,7 +246,7 @@ __attribute__((__packed__))
unsigned src1 : 13;
unsigned src2 : 13;
midgard_dest_override dest_override : 2;
- midgard_outmod outmod : 2;
+ midgard_outmod_float outmod : 2;
unsigned mask : 8;
}
midgard_vector_alu;
@@ -261,7 +268,7 @@ __attribute__((__packed__))
unsigned src1 : 6;
unsigned src2 : 11;
unsigned unknown : 1;
- midgard_outmod outmod : 2;
+ unsigned outmod : 2;
bool output_full : 1;
unsigned output_component : 3;
}
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index a0e4d05fd05..585c9fa7a75 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -540,14 +540,14 @@ emit_condition(compiler_context *ctx, nir_src *src, bool for_branch, unsigned co
.unit = for_branch ? UNIT_SMUL : UNIT_SADD,
.ssa_args = {
-
.src0 = condition,
.src1 = condition,
.dest = SSA_FIXED_REGISTER(31),
},
+
.alu = {
.op = midgard_alu_op_iand,
- .outmod = midgard_outmod_int,
+ .outmod = midgard_outmod_int_wrap,
.reg_mode = midgard_reg_mode_32,
.dest_override = midgard_dest_override_none,
.mask = (0x3 << 6), /* w */
@@ -586,7 +586,7 @@ emit_condition_mixed(compiler_context *ctx, nir_alu_src *src, unsigned nr_comp)
},
.alu = {
.op = midgard_alu_op_iand,
- .outmod = midgard_outmod_int,
+ .outmod = midgard_outmod_int_wrap,
.reg_mode = midgard_reg_mode_32,
.dest_override = midgard_dest_override_none,
.mask = expand_writemask((1 << nr_comp) - 1),
@@ -617,7 +617,7 @@ emit_indirect_offset(compiler_context *ctx, nir_src *src)
},
.alu = {
.op = midgard_alu_op_imov,
- .outmod = midgard_outmod_int,
+ .outmod = midgard_outmod_int_wrap,
.reg_mode = midgard_reg_mode_32,
.dest_override = midgard_dest_override_none,
.mask = (0x3 << 6), /* w */
@@ -820,12 +820,14 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
}
/* 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;
+ unsigned outmod;
- if (instr->op == nir_op_fsat)
- outmod = midgard_outmod_sat;
+ if (midgard_is_integer_out_op(op)) {
+ outmod = midgard_outmod_int_wrap;
+ } else {
+ bool sat = instr->dest.saturate || instr->op == nir_op_fsat;
+ outmod = sat ? midgard_outmod_sat : midgard_outmod_none;
+ }
/* fmax(a, 0.0) can turn into a .pos modifier as an optimization */
@@ -1753,6 +1755,18 @@ mir_nontrivial_source2_mod(midgard_instruction *ins)
return mir_nontrivial_mod(src2, is_int, mask);
}
+static bool
+mir_nontrivial_outmod(midgard_instruction *ins)
+{
+ bool is_int = midgard_is_integer_op(ins->alu.op);
+ unsigned mod = ins->alu.outmod;
+
+ if (is_int)
+ return mod != midgard_outmod_int_wrap;
+ else
+ return mod != midgard_outmod_none;
+}
+
static bool
midgard_opt_copy_prop(compiler_context *ctx, midgard_block *block)
{
@@ -1777,7 +1791,7 @@ midgard_opt_copy_prop(compiler_context *ctx, midgard_block *block)
if (ins->has_constants) continue;
if (mir_nontrivial_source2_mod(ins)) continue;
- if (ins->alu.outmod != midgard_outmod_none) continue;
+ if (mir_nontrivial_outmod(ins)) continue;
/* We're clear -- rewrite */
mir_rewrite_index_src(ctx, to, from);
@@ -1792,7 +1806,7 @@ midgard_opt_copy_prop(compiler_context *ctx, midgard_block *block)
* the move can be propagated away entirely */
static bool
-mir_compose_outmod(midgard_outmod *outmod, midgard_outmod comp)
+mir_compose_float_outmod(midgard_outmod_float *outmod, midgard_outmod_float comp)
{
/* Nothing to do */
if (comp == midgard_outmod_none)
@@ -1820,6 +1834,7 @@ midgard_opt_pos_propagate(compiler_context *ctx, midgard_block *block)
/* TODO: Registers? */
unsigned src = ins->ssa_args.src1;
if (src >= ctx->func->impl->ssa_alloc) continue;
+ assert(!mir_has_multiple_writes(ctx, src));
/* There might be a source modifier, too */
if (mir_nontrivial_source2_mod(ins)) continue;
@@ -1829,8 +1844,11 @@ midgard_opt_pos_propagate(compiler_context *ctx, midgard_block *block)
if (v->type != TAG_ALU_4) continue;
if (v->ssa_args.dest != src) continue;
- midgard_outmod temp = v->alu.outmod;
- progress |= mir_compose_outmod(&temp, ins->alu.outmod);
+ /* Can we even take a float outmod? */
+ if (midgard_is_integer_out_op(v->alu.op)) continue;
+
+ midgard_outmod_float temp = v->alu.outmod;
+ progress |= mir_compose_float_outmod(&temp, ins->alu.outmod);
/* Throw in the towel.. */
if (!progress) break;
@@ -2098,7 +2116,7 @@ emit_blend_epilogue(compiler_context *ctx)
.op = midgard_alu_op_imov,
.reg_mode = midgard_reg_mode_8,
.dest_override = midgard_dest_override_none,
- .outmod = midgard_outmod_int,
+ .outmod = midgard_outmod_int_wrap,
.mask = 0xFF,
.src1 = vector_alu_srco_unsigned(blank_alu_src),
.src2 = vector_alu_srco_unsigned(blank_alu_src),
--
2.20.1
More information about the mesa-dev
mailing list