panfrost/midgard: Add integer outmods

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@collabora.com>

diff --git a/src/gallium/drivers/panfrost/midgard/disassemble.c b/src/gallium/drivers/panfrost/midgard/disassemble.c
index a2a0b0dba376f258ad17f072f48ae20389407583..bf66d1a7da7ed1d79343f738035db649e4eafc7f 100644 (file)
--- a/src/gallium/drivers/panfrost/midgard/disassemble.c
+++ b/src/gallium/drivers/panfrost/midgard/disassemble.c
@@ -132,17 +132,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
@@ -377,7 +385,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;
@@ -503,7 +512,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 7bf17321ccb00a04c46383adf2721d05697c4193..ff853a197333e16a8830f3110e22a29c33459c36 100644 (file)
--- a/src/gallium/drivers/panfrost/midgard/midgard.h
+++ b/src/gallium/drivers/panfrost/midgard/midgard.h
@@ -172,15 +172,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 {
@@ -224,7 +231,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;
@@ -246,7 +253,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 af6f26855b02953c0e0874b41165a7e43b806d9e..bc9521aa9edc9609299ea52a5ba0373dc467f648 100644 (file)
--- 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 */
@@ -821,12 +821,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 */
 
@@ -1765,6 +1767,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)
 {
@@ -1789,7 +1803,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);
@@ -1804,7 +1818,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)
@@ -1832,6 +1846,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;
@@ -1841,8 +1856,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;
@@ -2076,7 +2094,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),