From 8b793f956712c277169b570dee6f9d4a2d2f9fda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 4 Feb 2021 16:01:44 +0100 Subject: [PATCH] aco: remove dead code for the handling of exec temporaries Totals from 26026 (18.67% of 139391) affected shaders (Navi10): PreSGPRs: 370993 -> 326177 (-12.08%) Reviewed-by: Rhys Perry Part-of: --- src/amd/compiler/aco_insert_exec_mask.cpp | 27 ++--------------- .../compiler/aco_instruction_selection.cpp | 5 ++-- src/amd/compiler/aco_ir.h | 1 - src/amd/compiler/aco_lower_to_cssa.cpp | 4 +-- src/amd/compiler/aco_register_allocation.cpp | 30 ------------------- src/amd/compiler/aco_spill.cpp | 4 --- src/amd/compiler/tests/helpers.cpp | 5 +--- src/amd/compiler/tests/helpers.h | 1 - src/amd/compiler/tests/test_builder.cpp | 2 +- src/amd/compiler/tests/test_optimizer.cpp | 24 +++++++-------- src/amd/compiler/tests/test_regalloc.cpp | 4 +-- 11 files changed, 22 insertions(+), 85 deletions(-) diff --git a/src/amd/compiler/aco_insert_exec_mask.cpp b/src/amd/compiler/aco_insert_exec_mask.cpp index e8e11abf77b..c2b0cea88c6 100644 --- a/src/amd/compiler/aco_insert_exec_mask.cpp +++ b/src/amd/compiler/aco_insert_exec_mask.cpp @@ -494,7 +494,6 @@ unsigned add_coupling_code(exec_ctx& ctx, Block* block, assert(!(block->kind & block_kind_top_level) || info.num_exec_masks <= 2); /* create the loop exit phis if not trivial */ - bool need_parallelcopy = false; for (unsigned exec_idx = 0; exec_idx < info.num_exec_masks; exec_idx++) { Temp same = ctx.info[preds[0]].exec[exec_idx].first; uint8_t type = ctx.info[header_preds[0]].exec[exec_idx].second; @@ -505,21 +504,6 @@ unsigned add_coupling_code(exec_ctx& ctx, Block* block, trivial = false; } - if (exec_idx == info.num_exec_masks - 1u) { - bool all_liveout_exec = true; - bool all_not_liveout_exec = true; - for (unsigned pred : preds) { - all_liveout_exec = all_liveout_exec && same == ctx.program->blocks[pred].live_out_exec; - all_not_liveout_exec = all_not_liveout_exec && same != ctx.program->blocks[pred].live_out_exec; - } - if (!all_liveout_exec && !all_not_liveout_exec) - trivial = false; - else if (all_not_liveout_exec) - need_parallelcopy = true; - - need_parallelcopy |= !trivial; - } - if (trivial) { ctx.info[idx].exec.emplace_back(same, type); } else { @@ -528,7 +512,6 @@ unsigned add_coupling_code(exec_ctx& ctx, Block* block, phi->definitions[0] = bld.def(bld.lm); if (exec_idx == info.num_exec_masks - 1u) { phi->definitions[0] = Definition(exec, bld.lm); - need_parallelcopy = false; } for (unsigned i = 0; i < phi->operands.size(); i++) phi->operands[i] = get_exec_op(ctx.info[preds[i]].exec[exec_idx].first); @@ -560,12 +543,8 @@ unsigned add_coupling_code(exec_ctx& ctx, Block* block, } assert(ctx.info[idx].exec.back().first.size() == bld.lm.size()); - if (need_parallelcopy && get_exec_op(ctx.info[idx].exec.back().first).isTemp()) { - /* only create this parallelcopy is needed, since the operand isn't - * fixed to exec which causes the spiller to miscalculate register demand */ - /* TODO: Fix register_demand calculation for spilling on loop exits. - * The problem is only mitigated because the register demand could be - * higher if the exec phi doesn't get assigned to exec. */ + if (get_exec_op(ctx.info[idx].exec.back().first).isTemp()) { + /* move current exec mask into exec register */ ctx.info[idx].exec.back().first = bld.pseudo(aco_opcode::p_parallelcopy, Definition(exec, bld.lm), ctx.info[idx].exec.back().first); } @@ -1062,8 +1041,6 @@ void process_block(exec_ctx& ctx, Block* block) block->instructions = std::move(instructions); add_branch_code(ctx, block); - - block->live_out_exec = ctx.info[block->index].exec.back().first; } } /* end namespace */ diff --git a/src/amd/compiler/aco_instruction_selection.cpp b/src/amd/compiler/aco_instruction_selection.cpp index 75c632ef851..efd7be9b679 100644 --- a/src/amd/compiler/aco_instruction_selection.cpp +++ b/src/amd/compiler/aco_instruction_selection.cpp @@ -11152,7 +11152,7 @@ Pseudo_instruction *add_startpgm(struct isel_context *ctx) } } - aco_ptr startpgm{create_instruction(aco_opcode::p_startpgm, Format::PSEUDO, 0, arg_count + 1)}; + aco_ptr startpgm{create_instruction(aco_opcode::p_startpgm, Format::PSEUDO, 0, arg_count)}; for (unsigned i = 0, arg = 0; i < ctx->args->ac.arg_count; i++) { if (ctx->args->ac.args[i].skip) continue; @@ -11167,7 +11167,6 @@ Pseudo_instruction *add_startpgm(struct isel_context *ctx) startpgm->definitions[arg].setFixed(PhysReg{file == AC_ARG_SGPR ? reg : reg + 256}); arg++; } - startpgm->definitions[arg_count] = Definition{ctx->program->allocateId(ctx->program->lane_mask), exec, ctx->program->lane_mask}; Pseudo_instruction *instr = startpgm.get(); ctx->block->instructions.push_back(std::move(startpgm)); @@ -11215,7 +11214,7 @@ void split_arguments(isel_context *ctx, Pseudo_instruction *startpgm) /* Split all arguments except for the first (ring_offsets) and the last * (exec) so that the dead channels don't stay live throughout the program. */ - for (int i = 1; i < startpgm->definitions.size() - 1; i++) { + for (int i = 1; i < startpgm->definitions.size(); i++) { if (startpgm->definitions[i].regClass().size() > 1) { emit_split_vector(ctx, startpgm->definitions[i].getTemp(), startpgm->definitions[i].regClass().size()); diff --git a/src/amd/compiler/aco_ir.h b/src/amd/compiler/aco_ir.h index d2fdf380117..d769f2062db 100644 --- a/src/amd/compiler/aco_ir.h +++ b/src/amd/compiler/aco_ir.h @@ -1673,7 +1673,6 @@ struct Block { uint16_t kind = 0; int logical_idom = -1; int linear_idom = -1; - Temp live_out_exec = Temp(); /* this information is needed for predecessors to blocks with phis when * moving out of ssa */ diff --git a/src/amd/compiler/aco_lower_to_cssa.cpp b/src/amd/compiler/aco_lower_to_cssa.cpp index accfd48f498..4babccf6580 100644 --- a/src/amd/compiler/aco_lower_to_cssa.cpp +++ b/src/amd/compiler/aco_lower_to_cssa.cpp @@ -99,8 +99,8 @@ bool collect_phi_info(cssa_ctx& ctx) if (op.isUndefined()) continue; /* check if the operand comes from the exec mask of a predecessor */ - if (op.isTemp() && op.getTemp() == ctx.program->blocks[preds[i]].live_out_exec) - op.setFixed(exec); + if (op.isFixed() && op.physReg() == exec) + continue; bool interferes = false; unsigned idom = is_logical ? diff --git a/src/amd/compiler/aco_register_allocation.cpp b/src/amd/compiler/aco_register_allocation.cpp index 2f17822e1f9..1363ff5bd55 100644 --- a/src/amd/compiler/aco_register_allocation.cpp +++ b/src/amd/compiler/aco_register_allocation.cpp @@ -2056,36 +2056,6 @@ void register_allocation(Program *program, std::vector& live_out_per_bloc /* this is a slight adjustment from the paper as we already have phi nodes: * We consider them incomplete phis and only handle the definition. */ - /* handle fixed phi definitions */ - for (instr_it = block.instructions.begin(); instr_it != block.instructions.end(); ++instr_it) { - aco_ptr& phi = *instr_it; - if (!is_phi(phi)) - break; - Definition& definition = phi->definitions[0]; - if (!definition.isFixed()) - continue; - - /* check if a dead exec mask phi is needed */ - if (definition.isKill()) { - for (Operand& op : phi->operands) { - assert(op.isTemp()); - if (!ctx.assignments[op.tempId()].assigned || - ctx.assignments[op.tempId()].reg != exec) { - definition.setKill(false); - break; - } - } - } - - if (definition.isKill()) - continue; - - assert(definition.physReg() == exec); - assert(!register_file.test(definition.physReg(), definition.bytes())); - register_file.fill(definition); - ctx.assignments[definition.tempId()] = {definition.physReg(), definition.regClass()}; - } - /* look up the affinities */ for (instr_it = block.instructions.begin(); instr_it != block.instructions.end(); ++instr_it) { aco_ptr& phi = *instr_it; diff --git a/src/amd/compiler/aco_spill.cpp b/src/amd/compiler/aco_spill.cpp index 2ea7265ca5a..22967abb472 100644 --- a/src/amd/compiler/aco_spill.cpp +++ b/src/amd/compiler/aco_spill.cpp @@ -189,8 +189,6 @@ void next_uses_per_block(spill_ctx& ctx, unsigned block_idx, std::set& block->logical_preds[i] : block->linear_preds[i]; if (instr->operands[i].isTemp()) { - if (instr->operands[i].getTemp() == ctx.program->blocks[pred_idx].live_out_exec) - continue; if (ctx.next_use_distances_end[pred_idx].find(instr->operands[i].getTemp()) == ctx.next_use_distances_end[pred_idx].end() || ctx.next_use_distances_end[pred_idx][instr->operands[i].getTemp()] != std::pair{block_idx, 0}) worklist.insert(pred_idx); @@ -208,8 +206,6 @@ void next_uses_per_block(spill_ctx& ctx, unsigned block_idx, std::set& uint32_t dom = pair.second.first; std::vector& preds = temp.is_linear() ? block->linear_preds : block->logical_preds; for (unsigned pred_idx : preds) { - if (temp == ctx.program->blocks[pred_idx].live_out_exec) - continue; if (ctx.program->blocks[pred_idx].loop_nest_depth > block->loop_nest_depth) distance += 0xFFFF; if (ctx.next_use_distances_end[pred_idx].find(temp) != ctx.next_use_distances_end[pred_idx].end()) { diff --git a/src/amd/compiler/tests/helpers.cpp b/src/amd/compiler/tests/helpers.cpp index bbb83ee9b65..d4224b3f3ae 100644 --- a/src/amd/compiler/tests/helpers.cpp +++ b/src/amd/compiler/tests/helpers.cpp @@ -42,7 +42,6 @@ radv_shader_info info; std::unique_ptr program; Builder bld(NULL); Temp inputs[16]; -Temp exec_input; static VkInstance instance_cache[CHIP_LAST] = {VK_NULL_HANDLE}; static VkDevice device_cache[CHIP_LAST] = {VK_NULL_HANDLE}; @@ -110,14 +109,12 @@ bool setup_cs(const char *input_spec, enum chip_class chip_class, if (input_spec) { unsigned num_inputs = DIV_ROUND_UP(strlen(input_spec), 3u); - aco_ptr startpgm{create_instruction(aco_opcode::p_startpgm, Format::PSEUDO, 0, num_inputs + 1)}; + aco_ptr startpgm{create_instruction(aco_opcode::p_startpgm, Format::PSEUDO, 0, num_inputs)}; for (unsigned i = 0; i < num_inputs; i++) { RegClass cls(input_spec[i * 3] == 'v' ? RegType::vgpr : RegType::sgpr, input_spec[i * 3 + 1] - '0'); inputs[i] = bld.tmp(cls); startpgm->definitions[i] = Definition(inputs[i]); } - exec_input = bld.tmp(program->lane_mask); - startpgm->definitions[num_inputs] = bld.exec(Definition(exec_input)); bld.insert(std::move(startpgm)); } diff --git a/src/amd/compiler/tests/helpers.h b/src/amd/compiler/tests/helpers.h index a81de09ff84..30ff8cab7e3 100644 --- a/src/amd/compiler/tests/helpers.h +++ b/src/amd/compiler/tests/helpers.h @@ -64,7 +64,6 @@ extern ac_shader_config config; extern radv_shader_info info; extern std::unique_ptr program; extern aco::Builder bld; -extern aco::Temp exec_input; extern aco::Temp inputs[16]; namespace aco { diff --git a/src/amd/compiler/tests/test_builder.cpp b/src/amd/compiler/tests/test_builder.cpp index ad6dff08b38..ee481ecdb21 100644 --- a/src/amd/compiler/tests/test_builder.cpp +++ b/src/amd/compiler/tests/test_builder.cpp @@ -27,7 +27,7 @@ using namespace aco; BEGIN_TEST(builder.v_mul_imm) for (unsigned i = GFX8; i <= GFX10; i++) { - //>> v1: %a, v1: %b, s1: %c, s1: %d, s2: %_:exec = p_startpgm + //>> v1: %a, v1: %b, s1: %c, s1: %d = p_startpgm if (!setup_cs("v1 v1 s1 s1", (chip_class)i)) continue; diff --git a/src/amd/compiler/tests/test_optimizer.cpp b/src/amd/compiler/tests/test_optimizer.cpp index 94105e3acd6..70b5ff53330 100644 --- a/src/amd/compiler/tests/test_optimizer.cpp +++ b/src/amd/compiler/tests/test_optimizer.cpp @@ -27,7 +27,7 @@ using namespace aco; BEGIN_TEST(optimize.neg) for (unsigned i = GFX9; i <= GFX10; i++) { - //>> v1: %a, v1: %b, s1: %c, s1: %d, s2: %_:exec = p_startpgm + //>> v1: %a, v1: %b, s1: %c, s1: %d = p_startpgm if (!setup_cs("v1 v1 s1 s1", (chip_class)i)) continue; @@ -82,7 +82,7 @@ BEGIN_TEST(optimize.neg) END_TEST BEGIN_TEST(optimize.output_modifiers) - //>> v1: %a, v1: %b, s2: %_:exec = p_startpgm + //>> v1: %a, v1: %b = p_startpgm if (!setup_cs("v1 v1", GFX9)) return; @@ -258,7 +258,7 @@ Temp create_subbrev_co(Operand op0, Operand op1, Operand op2) BEGIN_TEST(optimize.cndmask) for (unsigned i = GFX9; i <= GFX10; i++) { - //>> v1: %a, s1: %b, s2: %c, s2: %_:exec = p_startpgm + //>> v1: %a, s1: %b, s2: %c = p_startpgm if (!setup_cs("v1 s1 s2", (chip_class)i)) continue; @@ -301,7 +301,7 @@ END_TEST BEGIN_TEST(optimize.add_lshl) for (unsigned i = GFX8; i <= GFX10; i++) { - //>> s1: %a, v1: %b, s2: %_:exec = p_startpgm + //>> s1: %a, v1: %b = p_startpgm if (!setup_cs("s1 v1", (chip_class)i)) continue; @@ -388,7 +388,7 @@ Temp create_mad_u32_u16(Operand a, Operand b, Operand c, bool is16bit = true) BEGIN_TEST(optimize.mad_u32_u16) for (unsigned i = GFX9; i <= GFX10; i++) { - //>> v1: %a, v1: %b, s1: %c, s2: %_:exec = p_startpgm + //>> v1: %a, v1: %b, s1: %c = p_startpgm if (!setup_cs("v1 v1 s1", (chip_class)i)) continue; @@ -446,7 +446,7 @@ END_TEST BEGIN_TEST(optimize.bcnt) for (unsigned i = GFX8; i <= GFX10; i++) { - //>> v1: %a, s1: %b, s2: %_:exec = p_startpgm + //>> v1: %a, s1: %b = p_startpgm if (!setup_cs("v1 s1", (chip_class)i)) continue; @@ -533,7 +533,7 @@ BEGIN_TEST(optimize.clamp) aco_print_operand(&cfg.ub, output); fprintf(output, "\n"); - //>> v1: %a, v1: %b, v1: %c, s2: %_:exec = p_startpgm + //>> v1: %a, v1: %b, v1: %c = p_startpgm //! v1: %res0 = @med3 @ub, @lb, %a //! p_unit_test 0, %res0 @@ -600,7 +600,7 @@ BEGIN_TEST(optimize.clamp) END_TEST BEGIN_TEST(optimize.const_comparison_ordering) - //>> v1: %a, v1: %b, v2: %c, v1: %d, s2: %_:exec = p_startpgm + //>> v1: %a, v1: %b, v2: %c, v1: %d = p_startpgm if (!setup_cs("v1 v1 v2 v1", GFX9)) return; @@ -722,7 +722,7 @@ BEGIN_TEST(optimize.const_comparison_ordering) END_TEST BEGIN_TEST(optimize.add3) - //>> v1: %a, v1: %b, v1: %c, s2: %_:exec = p_startpgm + //>> v1: %a, v1: %b, v1: %c = p_startpgm if (!setup_cs("v1 v1 v1", GFX9)) return; @@ -751,7 +751,7 @@ END_TEST BEGIN_TEST(optimize.minmax) for (unsigned i = GFX8; i <= GFX10; i++) { - //>> v1: %a, s2: %_:exec = p_startpgm + //>> v1: %a = p_startpgm if (!setup_cs("v1", (chip_class)i)) continue; @@ -774,7 +774,7 @@ END_TEST BEGIN_TEST(optimize.mad_32_24) for (unsigned i = GFX8; i <= GFX9; i++) { - //>> v1: %a, v1: %b, v1: %c, s2: %_:exec = p_startpgm + //>> v1: %a, v1: %b, v1: %c = p_startpgm if (!setup_cs("v1 v1 v1", (chip_class)i)) continue; @@ -795,7 +795,7 @@ END_TEST BEGIN_TEST(optimize.add_lshlrev) for (unsigned i = GFX8; i <= GFX10; i++) { - //>> v1: %a, v1: %b, s1: %c, s2: %_:exec = p_startpgm + //>> v1: %a, v1: %b, s1: %c = p_startpgm if (!setup_cs("v1 v1 s1", (chip_class)i)) continue; diff --git a/src/amd/compiler/tests/test_regalloc.cpp b/src/amd/compiler/tests/test_regalloc.cpp index b6f07c06ae4..0bf0a237118 100644 --- a/src/amd/compiler/tests/test_regalloc.cpp +++ b/src/amd/compiler/tests/test_regalloc.cpp @@ -39,7 +39,7 @@ BEGIN_TEST(regalloc.subdword_alloc.reuse_16bit_operands) for (bool pessimistic : { false, true }) { const char* subvariant = pessimistic ? "/pessimistic" : "/optimistic"; - //>> v1: %_:v[#a], s2: %_:exec = p_startpgm + //>> v1: %_:v[#a] = p_startpgm if (!setup_cs("v1", (chip_class)cc, CHIP_UNKNOWN, subvariant)) return; @@ -60,7 +60,7 @@ BEGIN_TEST(regalloc.subdword_alloc.reuse_16bit_operands) END_TEST BEGIN_TEST(regalloc.32bit_partial_write) - //>> v1: %_:v[0], s2: %_:exec = p_startpgm + //>> v1: %_:v[0] = p_startpgm if (!setup_cs("v1", GFX10)) return;