panfrost/midgard: Use a union to manipulate embedded constants
authorBoris Brezillon <boris.brezillon@collabora.com>
Mon, 20 Jan 2020 14:00:57 +0000 (15:00 +0100)
committerMarge Bot <eric+marge@anholt.net>
Wed, 22 Jan 2020 15:31:28 +0000 (15:31 +0000)
Each instruction bundle can contain up to 16 constant bytes. The meaning
of those byte is instruction dependent: it depends on the instruction
native type (int, uint or float) and the instruction reg_mode (8, 16, 32
or 64 bit). Those different layouts can be exposed as a union to
facilitate constants manipulation.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3478>

src/panfrost/midgard/compiler.h
src/panfrost/midgard/midgard.h
src/panfrost/midgard/midgard_compile.c
src/panfrost/midgard/midgard_emit.c
src/panfrost/midgard/midgard_opt_float.c
src/panfrost/midgard/midgard_print.c
src/panfrost/midgard/midgard_schedule.c
src/panfrost/midgard/mir_promote_uniforms.c

index ab9cc819ef6aee927dd7e1455c3625fc7438207c..ef89589ced6219652b29548cc9e2430282f76e33 100644 (file)
@@ -103,7 +103,7 @@ typedef struct midgard_instruction {
         int unit;
 
         bool has_constants;
-        uint32_t constants[4];
+        midgard_constants constants;
         uint16_t inline_constant;
         bool has_blend_constant;
         bool has_inline_constant;
@@ -217,7 +217,7 @@ typedef struct midgard_bundle {
         int padding;
         int control;
         bool has_embedded_constants;
-        float constants[4];
+        midgard_constants constants;
         bool has_blend_constant;
         bool last_writeout;
 } midgard_bundle;
@@ -616,7 +616,7 @@ v_load_store_scratch(
                 .no_spill = (1 << REG_CLASS_WORK)
         };
 
-        ins.constants[0] = byte;
+        ins.constants.u32[0] = byte;
 
         if (is_store) {
                 ins.src[0] = srcdest;
index 909a2d12d2ca09fb167aac6eaa87cf266caa89c7..efb188f5b7ea92a41affeb972740a49333d81c09 100644 (file)
@@ -742,4 +742,23 @@ __attribute__((__packed__))
 }
 midgard_texture_word;
 
+/* Up to 16 constant bytes can be embedded in a bundle. This union describes
+ * all possible layouts.
+ */
+
+typedef union midgard_constants {
+        double f64[2];
+        uint64_t u64[2];
+        int64_t i64[2];
+        float f32[4];
+        uint32_t u32[4];
+        int32_t i32[4];
+        uint16_t f16[8];
+        uint16_t u16[8];
+        int16_t i16[8];
+        uint8_t u8[16];
+        int8_t i8[16];
+}
+midgard_constants;
+
 #endif
index d536f66449bcdccb8fc55d4dd219d6937405c164..bec94a037c903cb0f867c6ff8dc7fb83b32ccab7 100644 (file)
@@ -573,11 +573,33 @@ emit_load_const(compiler_context *ctx, nir_load_const_instr *instr)
 {
         nir_ssa_def def = instr->def;
 
-        float *v = rzalloc_array(NULL, float, 4);
-        nir_const_value_to_array(v, instr->value, instr->def.num_components, f32);
+        midgard_constants *consts = rzalloc(NULL, midgard_constants);
+
+        assert(instr->def.num_components * instr->def.bit_size <= sizeof(*consts) * 8);
+
+#define RAW_CONST_COPY(bits)                                         \
+        nir_const_value_to_array(consts->u##bits, instr->value,      \
+                                 instr->def.num_components, u##bits)
+
+        switch (instr->def.bit_size) {
+        case 64:
+                RAW_CONST_COPY(64);
+                break;
+        case 32:
+                RAW_CONST_COPY(32);
+                break;
+        case 16:
+                RAW_CONST_COPY(16);
+                break;
+        case 8:
+                RAW_CONST_COPY(8);
+                break;
+        default:
+                unreachable("Invalid bit_size for load_const instruction\n");
+        }
 
         /* Shifted for SSA, +1 for off-by-one */
-        _mesa_hash_table_u64_insert(ctx->ssa_constants, (def.index << 1) + 1, v);
+        _mesa_hash_table_u64_insert(ctx->ssa_constants, (def.index << 1) + 1, consts);
 }
 
 /* Normally constants are embedded implicitly, but for I/O and such we have to
@@ -1045,13 +1067,10 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
                 ins.src[1] = SSA_FIXED_REGISTER(REGISTER_CONSTANT);
                 ins.has_constants = true;
 
-                if (instr->op == nir_op_b2f32) {
-                        float f = 1.0f;
-                        memcpy(&ins.constants, &f, sizeof(float));
-                } else {
-                        ins.constants[0] = 1;
-                }
-
+                if (instr->op == nir_op_b2f32)
+                        ins.constants.f32[0] = 1.0f;
+                else
+                        ins.constants.i32[0] = 1;
 
                 for (unsigned c = 0; c < 16; ++c)
                         ins.swizzle[1][c] = 0;
@@ -1060,7 +1079,7 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
                 ins.has_inline_constant = false;
                 ins.src[1] = SSA_FIXED_REGISTER(REGISTER_CONSTANT);
                 ins.has_constants = true;
-                ins.constants[0] = 0;
+                ins.constants.u32[0] = 0;
 
                 for (unsigned c = 0; c < 16; ++c)
                         ins.swizzle[1][c] = 0;
@@ -1136,7 +1155,7 @@ emit_ubo_read(
         /* TODO: half-floats */
 
         midgard_instruction ins = m_ld_ubo_int4(dest, 0);
-        ins.constants[0] = offset;
+        ins.constants.u32[0] = offset;
 
         if (instr->type == nir_instr_type_intrinsic)
                 mir_set_intr_mask(instr, &ins, true);
@@ -1346,7 +1365,7 @@ emit_fragment_store(compiler_context *ctx, unsigned src, unsigned rt)
 
         /* Add dependencies */
         ins.src[0] = src;
-        ins.constants[0] = rt * 0x100;
+        ins.constants.u32[0] = rt * 0x100;
 
         /* Emit the branch */
         midgard_instruction *br = emit_mir_instruction(ctx, ins);
@@ -2159,16 +2178,16 @@ embedded_to_inline_constant(compiler_context *ctx, midgard_block *block)
                         /* Scale constant appropriately, if we can legally */
                         uint16_t scaled_constant = 0;
 
-                        if (midgard_is_integer_op(op) || is_16) {
-                                unsigned int *iconstants = (unsigned int *) ins->constants;
-                                scaled_constant = (uint16_t) iconstants[component];
+                        if (is_16) {
+                                scaled_constant = ins->constants.u16[component];
+                        } else if (midgard_is_integer_op(op)) {
+                                scaled_constant = ins->constants.u32[component];
 
                                 /* Constant overflow after resize */
-                                if (scaled_constant != iconstants[component])
+                                if (scaled_constant != ins->constants.u32[component])
                                         continue;
                         } else {
-                                float *f = (float *) ins->constants;
-                                float original = f[component];
+                                float original = ins->constants.f32[component];
                                 scaled_constant = _mesa_float_to_half(original);
 
                                 /* Check for loss of precision. If this is
@@ -2194,8 +2213,8 @@ embedded_to_inline_constant(compiler_context *ctx, midgard_block *block)
                         /* Make sure that the constant is not itself a vector
                          * by checking if all accessed values are the same. */
 
-                        uint32_t *cons = ins->constants;
-                        uint32_t value = cons[component];
+                        const midgard_constants *cons = &ins->constants;
+                        uint32_t value = is_16 ? cons->u16[component] : cons->u32[component];
 
                         bool is_vector = false;
                         unsigned mask = effective_writemask(&ins->alu, ins->mask);
@@ -2205,7 +2224,9 @@ embedded_to_inline_constant(compiler_context *ctx, midgard_block *block)
                                 if (!(mask & (1 << c)))
                                         continue;
 
-                                uint32_t test = cons[ins->swizzle[1][c]];
+                                uint32_t test = is_16 ?
+                                                cons->u16[ins->swizzle[1][c]] :
+                                                cons->u32[ins->swizzle[1][c]];
 
                                 if (test != value) {
                                         is_vector = true;
@@ -2312,7 +2333,7 @@ emit_fragment_epilogue(compiler_context *ctx, unsigned rt)
         struct midgard_instruction ins = v_branch(false, false);
         ins.writeout = true;
         ins.branch.target_block = ctx->block_count - 1;
-        ins.constants[0] = rt * 0x100;
+        ins.constants.u32[0] = rt * 0x100;
         emit_mir_instruction(ctx, ins);
 
         ctx->current_block->epilogue = true;
index 1dae8e390990e01f08375d3e3abf6f776627f8f8..7e948fd19c51054c33f6278eb939738694f066d7 100644 (file)
@@ -369,12 +369,8 @@ emit_alu_bundle(compiler_context *ctx,
 
         /* Tack on constants */
 
-        if (bundle->has_embedded_constants) {
-                util_dynarray_append(emission, float, bundle->constants[0]);
-                util_dynarray_append(emission, float, bundle->constants[1]);
-                util_dynarray_append(emission, float, bundle->constants[2]);
-                util_dynarray_append(emission, float, bundle->constants[3]);
-        }
+        if (bundle->has_embedded_constants)
+                util_dynarray_append(emission, midgard_constants, bundle->constants);
 }
 
 /* Shift applied to the immediate used as an offset. Probably this is papering
@@ -425,7 +421,7 @@ emit_binary_bundle(compiler_context *ctx,
                         mir_pack_swizzle_ldst(bundle->instructions[i]);
 
                         /* Apply a constant offset */
-                        unsigned offset = bundle->instructions[i]->constants[0];
+                        unsigned offset = bundle->instructions[i]->constants.u32[0];
 
                         if (offset) {
                                 unsigned shift = mir_ldst_imm_shift(bundle->instructions[i]->load_store.op);
index 39f95202857bd6b86a1701c3cff85222706f0c21..ab6e57be3f8e166371dd9c68d939a130f5d269fe 100644 (file)
@@ -60,8 +60,8 @@ midgard_opt_promote_fmov(compiler_context *ctx, midgard_block *block)
                 /* We found an imov with a constant. Check the constants */
                 bool ok = true;
 
-                for (unsigned i = 0; i < ARRAY_SIZE(ins->constants); ++i)
-                        ok &= mir_constant_float(ins->constants[i]);
+                for (unsigned i = 0; i < ARRAY_SIZE(ins->constants.u32); ++i)
+                        ok &= mir_constant_float(ins->constants.u32[i]);
 
                 if (!ok)
                         continue;
index 227b88c7055738576744c3f978a87422d068e3b8..43bb671dc66b79a50c114be92e93c0f32679b4bb 100644 (file)
@@ -171,8 +171,8 @@ mir_print_instruction(midgard_instruction *ins)
         mir_print_swizzle(ins->swizzle[3]);
 
         if (ins->has_constants) {
-                uint32_t *uc = ins->constants;
-                float *fc = (float *) uc;
+                uint32_t *uc = ins->constants.u32;
+                float *fc = ins->constants.f32;
 
                 if (midgard_is_integer_op(ins->alu.op))
                         printf(" <0x%X, 0x%X, 0x%X, 0x%x>", uc[0], uc[1], uc[2], uc[3]);
index f369b09e2ab7c928063e5097998e8a8fe4a37983..892f40d1aeb7072530974fc8d6bce6d95990a6b0 100644 (file)
@@ -332,7 +332,7 @@ struct midgard_predicate {
          * mode, the constants array will be updated, and the instruction
          * will be adjusted to index into the constants array */
 
-        uint8_t *constants;
+        midgard_constants *constants;
         unsigned constant_count;
         bool blend_constant;
 
@@ -378,8 +378,8 @@ mir_adjust_constants(midgard_instruction *ins,
                 if (pred->constant_count)
                         return false;
 
-                uint16_t *bundles = (uint16_t *) pred->constants;
-                uint32_t *constants = (uint32_t *) ins->constants;
+                uint16_t *bundles = pred->constants->u16;
+                const uint16_t *constants = ins->constants.u16;
 
                 /* Copy them wholesale */
                 for (unsigned i = 0; i < 4; ++i)
@@ -388,8 +388,8 @@ mir_adjust_constants(midgard_instruction *ins,
                 pred->constant_count = 16;
         } else {
                 /* Pack 32-bit constants */
-                uint32_t *bundles = (uint32_t *) pred->constants;
-                uint32_t *constants = (uint32_t *) ins->constants;
+                uint32_t *bundles = pred->constants->u32;
+                const uint32_t *constants = ins->constants.u32;
                 unsigned r_constant = SSA_FIXED_REGISTER(REGISTER_CONSTANT);
                 unsigned mask = mir_from_bytemask(mir_bytemask_of_read_components(ins, r_constant), midgard_reg_mode_32);
 
@@ -864,7 +864,7 @@ mir_schedule_alu(
                 .tag = TAG_ALU_4,
                 .destructive = true,
                 .exclude = ~0,
-                .constants = (uint8_t *) bundle.constants
+                .constants = &bundle.constants
         };
 
         midgard_instruction *vmul = NULL;
@@ -946,13 +946,13 @@ mir_schedule_alu(
 
         /* If we have a render target reference, schedule a move for it */
 
-        if (branch && branch->writeout && (branch->constants[0] || ctx->is_blend)) {
+        if (branch && branch->writeout && (branch->constants.u32[0] || ctx->is_blend)) {
                 midgard_instruction mov = v_mov(~0, make_compiler_temp(ctx));
                 sadd = mem_dup(&mov, sizeof(midgard_instruction));
                 sadd->unit = UNIT_SADD;
                 sadd->mask = 0x1;
                 sadd->has_inline_constant = true;
-                sadd->inline_constant = branch->constants[0];
+                sadd->inline_constant = branch->constants.u32[0];
                 branch->src[1] = mov.dest;
                 /* TODO: Don't leak */
         }
index 12f860787c1daa8f14deb1145a701971b9d90d90..2dbea712579a75da343a00618367df06caa63c34 100644 (file)
@@ -44,10 +44,10 @@ mir_is_promoteable_ubo(midgard_instruction *ins)
 
         return (ins->type == TAG_LOAD_STORE_4) &&
                 (OP_IS_UBO_READ(ins->load_store.op)) &&
-                !(ins->constants[0] & 0xF) &&
+                !(ins->constants.u32[0] & 0xF) &&
                 !(ins->load_store.arg_1) &&
                 (ins->load_store.arg_2 == 0x1E) &&
-                ((ins->constants[0] / 16) < 16);
+                ((ins->constants.u32[0] / 16) < 16);
 }
 
 static unsigned
@@ -57,7 +57,7 @@ mir_promoteable_uniform_count(compiler_context *ctx)
 
         mir_foreach_instr_global(ctx, ins) {
                 if (mir_is_promoteable_ubo(ins))
-                        count = MAX2(count, ins->constants[0] / 16);
+                        count = MAX2(count, ins->constants.u32[0] / 16);
         }
 
         return count;
@@ -135,7 +135,7 @@ midgard_promote_uniforms(compiler_context *ctx)
         mir_foreach_instr_global_safe(ctx, ins) {
                 if (!mir_is_promoteable_ubo(ins)) continue;
 
-                unsigned off = ins->constants[0];
+                unsigned off = ins->constants.u32[0];
                 unsigned address = off / 16;
 
                 /* Check if it's a promotable range */