From 15c92d158cad000b12cbed7f9c3a8248e8c99aee Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Mon, 20 Jan 2020 15:00:57 +0100 Subject: [PATCH] panfrost/midgard: Use a union to manipulate embedded constants 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 Reviewed-by: Alyssa Rosenzweig Part-of: --- src/panfrost/midgard/compiler.h | 6 +- src/panfrost/midgard/midgard.h | 19 ++++++ src/panfrost/midgard/midgard_compile.c | 67 ++++++++++++++------- src/panfrost/midgard/midgard_emit.c | 10 +-- src/panfrost/midgard/midgard_opt_float.c | 4 +- src/panfrost/midgard/midgard_print.c | 4 +- src/panfrost/midgard/midgard_schedule.c | 16 ++--- src/panfrost/midgard/mir_promote_uniforms.c | 8 +-- 8 files changed, 85 insertions(+), 49 deletions(-) diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h index ab9cc819ef6..ef89589ced6 100644 --- a/src/panfrost/midgard/compiler.h +++ b/src/panfrost/midgard/compiler.h @@ -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; diff --git a/src/panfrost/midgard/midgard.h b/src/panfrost/midgard/midgard.h index 909a2d12d2c..efb188f5b7e 100644 --- a/src/panfrost/midgard/midgard.h +++ b/src/panfrost/midgard/midgard.h @@ -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 diff --git a/src/panfrost/midgard/midgard_compile.c b/src/panfrost/midgard/midgard_compile.c index d536f66449b..bec94a037c9 100644 --- a/src/panfrost/midgard/midgard_compile.c +++ b/src/panfrost/midgard/midgard_compile.c @@ -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; diff --git a/src/panfrost/midgard/midgard_emit.c b/src/panfrost/midgard/midgard_emit.c index 1dae8e39099..7e948fd19c5 100644 --- a/src/panfrost/midgard/midgard_emit.c +++ b/src/panfrost/midgard/midgard_emit.c @@ -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); diff --git a/src/panfrost/midgard/midgard_opt_float.c b/src/panfrost/midgard/midgard_opt_float.c index 39f95202857..ab6e57be3f8 100644 --- a/src/panfrost/midgard/midgard_opt_float.c +++ b/src/panfrost/midgard/midgard_opt_float.c @@ -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; diff --git a/src/panfrost/midgard/midgard_print.c b/src/panfrost/midgard/midgard_print.c index 227b88c7055..43bb671dc66 100644 --- a/src/panfrost/midgard/midgard_print.c +++ b/src/panfrost/midgard/midgard_print.c @@ -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]); diff --git a/src/panfrost/midgard/midgard_schedule.c b/src/panfrost/midgard/midgard_schedule.c index f369b09e2ab..892f40d1aeb 100644 --- a/src/panfrost/midgard/midgard_schedule.c +++ b/src/panfrost/midgard/midgard_schedule.c @@ -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 */ } diff --git a/src/panfrost/midgard/mir_promote_uniforms.c b/src/panfrost/midgard/mir_promote_uniforms.c index 12f860787c1..2dbea712579 100644 --- a/src/panfrost/midgard/mir_promote_uniforms.c +++ b/src/panfrost/midgard/mir_promote_uniforms.c @@ -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 */ -- 2.30.2