From c908772ee47201a1f84503099d1e0767a9f9818f Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Thu, 1 Aug 2019 13:29:01 -0700 Subject: [PATCH] pan/midgard: Split ld/st unknown to arg_1/arg_2 fields The 16-bit field can be decomposed to two independent 8-bit fields, each representing a single (additional) argument to the load/store op, generally used for encoding registers. Addressable registers here are substantially limited compared to the main register in a load/store op. Signed-off-by: Alyssa Rosenzweig --- src/panfrost/midgard/disassemble.c | 12 ++++++++++- src/panfrost/midgard/midgard.h | 9 ++++++++- src/panfrost/midgard/midgard_compile.c | 20 ++++++++++++------- src/panfrost/midgard/midgard_liveness.c | 6 +++++- .../midgard/midgard_opt_perspective.c | 2 +- src/panfrost/midgard/midgard_schedule.c | 10 ++++++---- src/panfrost/midgard/mir_promote_uniforms.c | 4 ++-- 7 files changed, 46 insertions(+), 17 deletions(-) diff --git a/src/panfrost/midgard/disassemble.c b/src/panfrost/midgard/disassemble.c index a74280fc733..b9740caa1b8 100644 --- a/src/panfrost/midgard/disassemble.c +++ b/src/panfrost/midgard/disassemble.c @@ -968,6 +968,12 @@ is_op_varying(unsigned op) return false; } +static void +print_load_store_arg(uint8_t arg) +{ + printf("0x%X", arg); +} + static void print_load_store_instr(uint64_t data, unsigned tabs) @@ -998,7 +1004,11 @@ print_load_store_instr(uint64_t data, print_swizzle_vec4(word->swizzle, false, false); - printf(", 0x%X /* %X */\n", word->unknown, word->varying_parameters); + printf(", "); + print_load_store_arg(word->arg_1); + printf(", "); + print_load_store_arg(word->arg_2); + printf(" /* %X */\n", word->varying_parameters); } static void diff --git a/src/panfrost/midgard/midgard.h b/src/panfrost/midgard/midgard.h index aa8b1793c99..45eaca21c42 100644 --- a/src/panfrost/midgard/midgard.h +++ b/src/panfrost/midgard/midgard.h @@ -505,7 +505,14 @@ __attribute__((__packed__)) unsigned reg : 5; unsigned mask : 4; unsigned swizzle : 8; - unsigned unknown : 16; + + /* Load/store ops can take two additional registers as arguments, but + * these are limited to load/store registers with only a few supported + * mask/swizzle combinations. The tradeoff is these are much more + * compact, requiring 8-bits each rather than 17-bits for a full + * reg/mask/swizzle */ + unsigned arg_1 : 8; + unsigned arg_2 : 8; unsigned varying_parameters : 10; diff --git a/src/panfrost/midgard/midgard_compile.c b/src/panfrost/midgard/midgard_compile.c index 23a05607847..7314e678c8c 100644 --- a/src/panfrost/midgard/midgard_compile.c +++ b/src/panfrost/midgard/midgard_compile.c @@ -1164,11 +1164,13 @@ emit_ubo_read( if (indirect_offset) { emit_indirect_offset(ctx, indirect_offset); - ins.load_store.unknown = 0x8700 | index; /* xxx: what is this? */ + ins.load_store.arg_2 = 0x87; } else { - ins.load_store.unknown = 0x1E00 | index; /* xxx: what is this? */ + ins.load_store.arg_2 = 0x1E; } + ins.load_store.arg_1 = index; + emit_mir_instruction(ctx, ins); } @@ -1199,12 +1201,14 @@ emit_varying_read( if (indirect_offset) { /* We need to add in the dynamic index, moved to r27.w */ emit_indirect_offset(ctx, indirect_offset); - ins.load_store.unknown = 0x79e; /* xxx: what is this? */ + ins.load_store.arg_2 = 0x07; } else { /* Just a direct load */ - ins.load_store.unknown = 0x1e9e; /* xxx: what is this? */ + ins.load_store.arg_2 = 0x1E; } + ins.load_store.arg_1 = 0x9E; + /* Use the type appropriate load */ switch (type) { case nir_type_uint: @@ -1320,7 +1324,8 @@ emit_intrinsic(compiler_context *ctx, nir_intrinsic_instr *instr) emit_mir_instruction(ctx, move); } else if (ctx->stage == MESA_SHADER_VERTEX) { midgard_instruction ins = m_ld_attr_32(reg, offset); - ins.load_store.unknown = 0x1E1E; /* XXX: What is this? */ + ins.load_store.arg_1 = 0x1E; + ins.load_store.arg_2 = 0x1E; ins.mask = mask_of(nr_comp); /* Use the type appropriate load */ @@ -1408,7 +1413,8 @@ emit_intrinsic(compiler_context *ctx, nir_intrinsic_instr *instr) unsigned component = nir_intrinsic_component(instr); midgard_instruction st = m_st_vary_32(reg, offset); - st.load_store.unknown = 0x1E9E; /* XXX: What is this? */ + st.load_store.arg_1 = 0x9E; + st.load_store.arg_2 = 0x1E; st.load_store.swizzle = SWIZZLE_XYZW << (2*component); emit_mir_instruction(ctx, st); } else { @@ -1604,7 +1610,7 @@ emit_texop_native(compiler_context *ctx, nir_tex_instr *instr, midgard_instruction st = m_st_cubemap_coords(temp, 0); st.ssa_args.src0 = index; - st.load_store.unknown = 0x24; /* XXX: What is this? */ + st.load_store.arg_1 = 0x24; st.mask = 0x3; /* xy */ st.load_store.swizzle = alu_src.swizzle; emit_mir_instruction(ctx, st); diff --git a/src/panfrost/midgard/midgard_liveness.c b/src/panfrost/midgard/midgard_liveness.c index e3fa2520acf..39f00c09651 100644 --- a/src/panfrost/midgard/midgard_liveness.c +++ b/src/panfrost/midgard/midgard_liveness.c @@ -66,7 +66,11 @@ is_live_after_successors(compiler_context *ctx, midgard_block *bl, int src) /* If written-before-use, we're gone */ - if (ins->ssa_args.dest == src && ins->type == TAG_LOAD_STORE_4 && ins->load_store.op == midgard_op_ld_int4 && ins->load_store.unknown == 0x1EEA) { + if (ins->ssa_args.dest == src && + ins->type == TAG_LOAD_STORE_4 && + ins->load_store.op == midgard_op_ld_int4 && + ins->load_store.arg_1 == 0xEA && + ins->load_store.arg_2 == 0x1E) { block_done = true; break; } diff --git a/src/panfrost/midgard/midgard_opt_perspective.c b/src/panfrost/midgard/midgard_opt_perspective.c index 3ab94deed58..fe816481fef 100644 --- a/src/panfrost/midgard/midgard_opt_perspective.c +++ b/src/panfrost/midgard/midgard_opt_perspective.c @@ -124,7 +124,7 @@ midgard_opt_combine_projection(compiler_context *ctx, midgard_block *block) midgard_op_ldst_perspective_division_w : midgard_op_ldst_perspective_division_z, .swizzle = SWIZZLE_XYZW, - .unknown = 0x24, + .arg_1 = 0x24, } }; diff --git a/src/panfrost/midgard/midgard_schedule.c b/src/panfrost/midgard/midgard_schedule.c index 7ad45f2a066..cb4757a9a83 100644 --- a/src/panfrost/midgard/midgard_schedule.c +++ b/src/panfrost/midgard/midgard_schedule.c @@ -630,11 +630,11 @@ midgard_pair_load_store(compiler_context *ctx, midgard_block *block) if (OP_IS_STORE(c->load_store.op)) continue; - /* It appears the 0x800 bit is set whenever a + /* It appears the 0x8 bit is set whenever a * load is direct, unset when it is indirect. * Skip indirect loads. */ - if (!(c->load_store.unknown & 0x800)) continue; + if (!(c->load_store.arg_2 & 0x8)) continue; /* We found one! Move it up to pair and remove it from the old location */ @@ -712,7 +712,8 @@ v_load_store_scratch( .swizzle = SWIZZLE_XYZW, /* For register spilling - to thread local storage */ - .unknown = 0x1EEA, + .arg_1 = 0xEA, + .arg_2 = 0x1E, /* Splattered across, TODO combine logically */ .varying_parameters = (byte & 0x1FF) << 1, @@ -772,7 +773,8 @@ schedule_program(compiler_context *ctx) mir_foreach_instr_global(ctx, ins) { if (ins->type != TAG_LOAD_STORE_4) continue; if (ins->load_store.op != midgard_op_ld_int4) continue; - if (ins->load_store.unknown != 0x1EEA) continue; + if (ins->load_store.arg_1 != 0xEA) continue; + if (ins->load_store.arg_2 != 0x1E) continue; ra_set_node_spill_cost(g, ins->ssa_args.dest, -1.0); } diff --git a/src/panfrost/midgard/mir_promote_uniforms.c b/src/panfrost/midgard/mir_promote_uniforms.c index 348fa31f3c7..6c09fbe661e 100644 --- a/src/panfrost/midgard/mir_promote_uniforms.c +++ b/src/panfrost/midgard/mir_promote_uniforms.c @@ -53,10 +53,10 @@ midgard_promote_uniforms(compiler_context *ctx, unsigned register_pressure) unsigned address = (hi << 3) | lo; /* Check this is UBO 0 */ - if (ins->load_store.unknown & 0xF) continue; + if (ins->load_store.arg_1) continue; /* Check we're accessing directly */ - if (ins->load_store.unknown != 0x1E00) continue; + if (ins->load_store.arg_2 != 0x1E) continue; /* Check if it's a promotable range */ unsigned uniform_reg = 23 - address; -- 2.30.2