From dfa38fa0c7034d71c2a21ae7261c73ed9880f783 Mon Sep 17 00:00:00 2001 From: Rhys Perry Date: Mon, 30 Nov 2020 17:53:23 +0000 Subject: [PATCH] aco: group loads from the same vertex binding into the same clause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the future, we might have vertex attribute loads from the same binding but with different descriptors. Since they will be loading from the same buffer, we should continue grouping them into clauses. No fossil-db changes. Signed-off-by: Rhys Perry Reviewed-by: Daniel Schürmann Part-of: --- src/amd/compiler/aco_form_hard_clauses.cpp | 18 +++++-------- .../compiler/aco_instruction_selection.cpp | 10 ++++--- src/amd/compiler/aco_ir.cpp | 26 +++++++++++++++++++ src/amd/compiler/aco_ir.h | 8 ++++-- src/amd/compiler/aco_scheduler.cpp | 6 ++--- src/amd/compiler/aco_statistics.cpp | 26 +++++++++---------- 6 files changed, 58 insertions(+), 36 deletions(-) diff --git a/src/amd/compiler/aco_form_hard_clauses.cpp b/src/amd/compiler/aco_form_hard_clauses.cpp index e04895a238c..13e9dfadc3d 100644 --- a/src/amd/compiler/aco_form_hard_clauses.cpp +++ b/src/amd/compiler/aco_form_hard_clauses.cpp @@ -64,7 +64,6 @@ void form_hard_clauses(Program *program) unsigned num_instrs = 0; aco_ptr current_instrs[64]; clause_type current_type = clause_other; - unsigned current_resource = 0; std::vector> new_instructions; new_instructions.reserve(block.instructions.size()); @@ -73,26 +72,21 @@ void form_hard_clauses(Program *program) for (unsigned i = 0; i < block.instructions.size(); i++) { aco_ptr& instr = block.instructions[i]; - unsigned resource = 0; clause_type type = clause_other; - if (instr->isVMEM() && !instr->operands.empty()) { - resource = instr->operands[0].tempId(); + if (instr->isVMEM() && !instr->operands.empty()) type = clause_vmem; - } else if (instr->isScratch() || instr->isGlobal()) { + else if (instr->isScratch() || instr->isGlobal()) type = clause_vmem; - } else if (instr->isFlat()) { + else if (instr->isFlat()) type = clause_flat; - } else if (instr->isSMEM() && !instr->operands.empty()) { + else if (instr->isSMEM() && !instr->operands.empty()) type = clause_smem; - if (instr->operands[0].bytes() == 16) - resource = instr->operands[0].tempId(); - } - if (type != current_type || resource != current_resource || num_instrs == 64) { + if (type != current_type || num_instrs == 64 || + (num_instrs && !should_form_clause(current_instrs[0].get(), instr.get()))) { emit_clause(bld, num_instrs, current_instrs); num_instrs = 0; current_type = type; - current_resource = resource; } if (type == clause_other) { diff --git a/src/amd/compiler/aco_instruction_selection.cpp b/src/amd/compiler/aco_instruction_selection.cpp index e57f60b3ef9..e1112a0d33f 100644 --- a/src/amd/compiler/aco_instruction_selection.cpp +++ b/src/amd/compiler/aco_instruction_selection.cpp @@ -4815,13 +4815,15 @@ void visit_load_input(isel_context *ctx, nir_intrinsic_instr *instr) } if (use_mubuf) { - bld.mubuf(opcode, - Definition(fetch_dst), list, fetch_index, soffset, + Instruction *mubuf = bld.mubuf( + opcode, Definition(fetch_dst), list, fetch_index, soffset, fetch_offset, false, false, true).instr; + mubuf->mubuf().vtx_binding = attrib_binding + 1; } else { - bld.mtbuf(opcode, - Definition(fetch_dst), list, fetch_index, soffset, + Instruction *mtbuf = bld.mtbuf( + opcode, Definition(fetch_dst), list, fetch_index, soffset, fetch_dfmt, nfmt, fetch_offset, false, true).instr; + mtbuf->mtbuf().vtx_binding = attrib_binding + 1; } emit_split_vector(ctx, fetch_dst, fetch_dst.size()); diff --git a/src/amd/compiler/aco_ir.cpp b/src/amd/compiler/aco_ir.cpp index 36855a84c97..458bf7981e4 100644 --- a/src/amd/compiler/aco_ir.cpp +++ b/src/amd/compiler/aco_ir.cpp @@ -544,4 +544,30 @@ bool wait_imm::empty() const lgkm == unset_counter && vs == unset_counter; } +bool should_form_clause(const Instruction *a, const Instruction *b) +{ + /* Vertex attribute loads from the same binding likely load from similar addresses */ + unsigned a_vtx_binding = a->isMUBUF() ? a->mubuf().vtx_binding : (a->isMTBUF() ? a->mtbuf().vtx_binding : 0); + unsigned b_vtx_binding = b->isMUBUF() ? b->mubuf().vtx_binding : (b->isMTBUF() ? b->mtbuf().vtx_binding : 0); + if (a_vtx_binding && a_vtx_binding == b_vtx_binding) + return true; + + if (a->format != b->format) + return false; + + /* Assume loads which don't use descriptors might load from similar addresses. */ + if (a->isFlatLike()) + return true; + if (a->isSMEM() && a->operands[0].bytes() == 8 && b->operands[0].bytes() == 8) + return true; + + /* If they load from the same descriptor, assume they might load from similar + * addresses. + */ + if (a->isVMEM() || a->isSMEM()) + return a->operands[0].tempId() == b->operands[0].tempId(); + + return false; +} + } diff --git a/src/amd/compiler/aco_ir.h b/src/amd/compiler/aco_ir.h index 2013567f993..3a9cb425895 100644 --- a/src/amd/compiler/aco_ir.h +++ b/src/amd/compiler/aco_ir.h @@ -1392,7 +1392,8 @@ struct MUBUF_instruction : public Instruction { uint16_t offset : 12; /* Unsigned byte offset - 12 bit */ uint16_t swizzled : 1; uint16_t padding0 : 2; - uint16_t padding1; + uint16_t vtx_binding : 6; /* 0 if this is not a vertex attribute load */ + uint16_t padding1 : 10; }; static_assert(sizeof(MUBUF_instruction) == sizeof(Instruction) + 8, "Unexpected padding"); @@ -1415,7 +1416,8 @@ struct MTBUF_instruction : public Instruction { uint16_t slc : 1; /* system level coherent */ uint16_t tfe : 1; /* texture fail enable */ uint16_t disable_wqm : 1; /* Require an exec mask without helper invocations */ - uint16_t padding : 10; + uint16_t vtx_binding : 6; /* 0 if this is not a vertex attribute load */ + uint16_t padding : 4; uint16_t offset; /* Unsigned byte offset - 12 bit */ }; static_assert(sizeof(MTBUF_instruction) == sizeof(Instruction) + 8, "Unexpected padding"); @@ -1618,6 +1620,8 @@ uint32_t get_reduction_identity(ReduceOp op, unsigned idx); unsigned get_mimg_nsa_dwords(const Instruction *instr); +bool should_form_clause(const Instruction *a, const Instruction *b); + enum block_kind { /* uniform indicates that leaving this block, * all actives lanes stay active */ diff --git a/src/amd/compiler/aco_scheduler.cpp b/src/amd/compiler/aco_scheduler.cpp index e3bce9a4b11..f40ea7468c3 100644 --- a/src/amd/compiler/aco_scheduler.cpp +++ b/src/amd/compiler/aco_scheduler.cpp @@ -692,13 +692,11 @@ void schedule_VMEM(sched_ctx& ctx, Block* block, bool part_of_clause = false; if (current->isVMEM() == candidate->isVMEM()) { - bool same_resource = true; - if (current->isVMEM()) - same_resource = candidate->operands[0].tempId() == current->operands[0].tempId(); int grab_dist = ctx.mv.insert_idx_clause - candidate_idx; /* We can't easily tell how much this will decrease the def-to-use * distances, so just use how far it will be moved as a heuristic. */ - part_of_clause = same_resource && grab_dist < clause_max_grab_dist; + part_of_clause = grab_dist < clause_max_grab_dist && + should_form_clause(current, candidate.get()); } /* if current depends on candidate, add additional dependencies and continue */ diff --git a/src/amd/compiler/aco_statistics.cpp b/src/amd/compiler/aco_statistics.cpp index 87a4f0773e2..f74206b9b55 100644 --- a/src/amd/compiler/aco_statistics.cpp +++ b/src/amd/compiler/aco_statistics.cpp @@ -445,8 +445,8 @@ void BlockCycleEstimator::join(const BlockCycleEstimator& pred) void collect_preasm_stats(Program *program) { for (Block& block : program->blocks) { - std::set vmem_clause_res; - std::set smem_clause_res; + std::set vmem_clause; + std::set smem_clause; program->statistics[statistic_instructions] += block.instructions.size(); @@ -458,25 +458,23 @@ void collect_preasm_stats(Program *program) program->statistics[statistic_instructions] += 2; if (instr->isVMEM() && !instr->operands.empty()) { - vmem_clause_res.insert(instr->operands[0].getTemp()); + if (std::none_of(vmem_clause.begin(), vmem_clause.end(), + [&](Instruction *other) {return should_form_clause(instr.get(), other);})) + program->statistics[statistic_vmem_clauses]++; + vmem_clause.insert(instr.get()); } else { - program->statistics[statistic_vmem_clauses] += vmem_clause_res.size(); - vmem_clause_res.clear(); + vmem_clause.clear(); } if (instr->isSMEM() && !instr->operands.empty()) { - if (instr->operands[0].size() == 2) - smem_clause_res.insert(Temp(0, s2)); - else - smem_clause_res.insert(instr->operands[0].getTemp()); + if (std::none_of(smem_clause.begin(), smem_clause.end(), + [&](Instruction *other) {return should_form_clause(instr.get(), other);})) + program->statistics[statistic_smem_clauses]++; + smem_clause.insert(instr.get()); } else { - program->statistics[statistic_smem_clauses] += smem_clause_res.size(); - smem_clause_res.clear(); + smem_clause.clear(); } } - - program->statistics[statistic_vmem_clauses] += vmem_clause_res.size(); - program->statistics[statistic_smem_clauses] += smem_clause_res.size(); } double latency = 0;