From 7b04c13a344fa5f2fed07a75191d680269f599d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 8 Sep 2021 11:56:52 +0200 Subject: [PATCH] aco/ra: don't rewrite affinities for phi operands after register assignment The effect of doing so is random and not meaningful. Totals from 52 (0.03% of 150170) affected shaders: (GFX10.3) CodeSize: 538768 -> 538784 (+0.00%); split: -0.04%, +0.04% Instrs: 100661 -> 100707 (+0.05%); split: -0.01%, +0.06% Latency: 1205950 -> 1205768 (-0.02%); split: -0.07%, +0.05% InvThroughput: 200106 -> 200040 (-0.03%); split: -0.31%, +0.28% Copies: 5717 -> 5754 (+0.65%); split: -0.17%, +0.82% Branches: 3153 -> 3162 (+0.29%) Reviewed-by: Rhys Perry Part-of: --- src/amd/compiler/aco_register_allocation.cpp | 110 +++++++++---------- 1 file changed, 53 insertions(+), 57 deletions(-) diff --git a/src/amd/compiler/aco_register_allocation.cpp b/src/amd/compiler/aco_register_allocation.cpp index a8d4668426d..a037745aaaf 100644 --- a/src/amd/compiler/aco_register_allocation.cpp +++ b/src/amd/compiler/aco_register_allocation.cpp @@ -1987,69 +1987,65 @@ get_regs_for_phis(ra_ctx& ctx, Block& block, RegisterFile& register_file, if (definition.isKill()) continue; - if (!definition.isFixed()) { - std::vector> parallelcopy; - definition.setFixed(get_reg(ctx, register_file, definition.getTemp(), parallelcopy, phi)); - update_renames(ctx, register_file, parallelcopy, phi, rename_not_killed_ops); + if (definition.isFixed()) { + instructions.emplace_back(std::move(phi)); + continue; + } - /* process parallelcopy */ - for (std::pair pc : parallelcopy) { - /* see if it's a copy from a different phi */ - // TODO: prefer moving some previous phis over live-ins - // TODO: somehow prevent phis fixed before the RA from being updated (shouldn't be a - // problem in practice since they can only be fixed to exec) - Instruction* prev_phi = NULL; - std::vector>::iterator phi_it; - for (phi_it = instructions.begin(); phi_it != instructions.end(); ++phi_it) { - if ((*phi_it)->definitions[0].tempId() == pc.first.tempId()) - prev_phi = phi_it->get(); - } - if (prev_phi) { - /* if so, just update that phi's register */ - prev_phi->definitions[0].setFixed(pc.second.physReg()); - ctx.assignments[prev_phi->definitions[0].tempId()].set(pc.second); - continue; - } + std::vector> parallelcopy; + definition.setFixed(get_reg(ctx, register_file, definition.getTemp(), parallelcopy, phi)); + update_renames(ctx, register_file, parallelcopy, phi, rename_not_killed_ops); - /* rename */ - std::unordered_map::iterator orig_it = - ctx.orig_names.find(pc.first.tempId()); - Temp orig = pc.first.getTemp(); - if (orig_it != ctx.orig_names.end()) - orig = orig_it->second; - else - ctx.orig_names[pc.second.tempId()] = orig; - ctx.renames[block.index][orig.id()] = pc.second.getTemp(); - - /* otherwise, this is a live-in and we need to create a new phi - * to move it in this block's predecessors */ - aco_opcode opcode = - pc.first.getTemp().is_linear() ? aco_opcode::p_linear_phi : aco_opcode::p_phi; - std::vector& preds = - pc.first.getTemp().is_linear() ? block.linear_preds : block.logical_preds; - aco_ptr new_phi{ - create_instruction(opcode, Format::PSEUDO, preds.size(), 1)}; - new_phi->definitions[0] = pc.second; - for (unsigned i = 0; i < preds.size(); i++) - new_phi->operands[i] = Operand(pc.first); - instructions.emplace_back(std::move(new_phi)); - - /* Remove from live_out_per_block (now used for live-in), because handle_loop_phis() - * would re-create this phi later if this is a loop header. - */ - live_in.erase(orig.id()); + /* process parallelcopy */ + for (std::pair pc : parallelcopy) { + /* see if it's a copy from a different phi */ + // TODO: prefer moving some previous phis over live-ins + // TODO: somehow prevent phis fixed before the RA from being updated (shouldn't be a + // problem in practice since they can only be fixed to exec) + Instruction* prev_phi = NULL; + std::vector>::iterator phi_it; + for (phi_it = instructions.begin(); phi_it != instructions.end(); ++phi_it) { + if ((*phi_it)->definitions[0].tempId() == pc.first.tempId()) + prev_phi = phi_it->get(); + } + if (prev_phi) { + /* if so, just update that phi's register */ + prev_phi->definitions[0].setFixed(pc.second.physReg()); + ctx.assignments[prev_phi->definitions[0].tempId()].set(pc.second); + continue; } - register_file.fill(definition); - ctx.assignments[definition.tempId()].set(definition); - } - - /* update phi affinities */ - for (const Operand& op : phi->operands) { - if (op.isTemp() && op.regClass() == phi->definitions[0].regClass()) - ctx.assignments[op.tempId()].affinity = definition.tempId(); + /* rename */ + std::unordered_map::iterator orig_it = + ctx.orig_names.find(pc.first.tempId()); + Temp orig = pc.first.getTemp(); + if (orig_it != ctx.orig_names.end()) + orig = orig_it->second; + else + ctx.orig_names[pc.second.tempId()] = orig; + ctx.renames[block.index][orig.id()] = pc.second.getTemp(); + + /* otherwise, this is a live-in and we need to create a new phi + * to move it in this block's predecessors */ + aco_opcode opcode = + pc.first.getTemp().is_linear() ? aco_opcode::p_linear_phi : aco_opcode::p_phi; + std::vector& preds = + pc.first.getTemp().is_linear() ? block.linear_preds : block.logical_preds; + aco_ptr new_phi{ + create_instruction(opcode, Format::PSEUDO, preds.size(), 1)}; + new_phi->definitions[0] = pc.second; + for (unsigned i = 0; i < preds.size(); i++) + new_phi->operands[i] = Operand(pc.first); + instructions.emplace_back(std::move(new_phi)); + + /* Remove from live_out_per_block (now used for live-in), because handle_loop_phis() + * would re-create this phi later if this is a loop header. + */ + live_in.erase(orig.id()); } + register_file.fill(definition); + ctx.assignments[definition.tempId()].set(definition); instructions.emplace_back(std::move(phi)); } }