From 2c6d3dde672e54aab236e49e903b7f16ba686384 Mon Sep 17 00:00:00 2001 From: kd-11 Date: Tue, 9 Dec 2025 03:04:03 +0300 Subject: [PATCH] C++ Pro - Different from C++ amateur. - Addresses code review suggestions to step up the C++ --- rpcs3/Emu/RSX/Program/Assembler/FPASM.cpp | 53 ++++++++----------- rpcs3/Emu/RSX/Program/Assembler/FPASM.h | 2 +- .../Passes/FP/RegisterAnnotationPass.cpp | 23 +++----- .../Passes/FP/RegisterDependencyPass.cpp | 22 ++++---- 4 files changed, 43 insertions(+), 57 deletions(-) diff --git a/rpcs3/Emu/RSX/Program/Assembler/FPASM.cpp b/rpcs3/Emu/RSX/Program/Assembler/FPASM.cpp index 4be6acb55b..ee8f4441cb 100644 --- a/rpcs3/Emu/RSX/Program/Assembler/FPASM.cpp +++ b/rpcs3/Emu/RSX/Program/Assembler/FPASM.cpp @@ -19,7 +19,7 @@ namespace rsx::assembler bool set_cond; }; - static std::unordered_map s_opcode_lookup + static std::unordered_map s_opcode_lookup { // Arithmetic { "NOP", { .op = RSX_FP_OPCODE_NOP, .exec_if_lt = true, .exec_if_eq = true, .exec_if_gt = true, .set_cond = false } }, @@ -168,10 +168,9 @@ namespace rsx::assembler std::vector result; result.reserve(m_instructions.size() * 4); - for (u32 i = 0; i < m_instructions.size(); ++i) + for (const auto& inst : m_instructions) { - const auto& inst = m_instructions[i]; - auto src = reinterpret_cast*>(inst.bytecode); + const auto src = reinterpret_cast*>(inst.bytecode); for (u32 j = 0; j < inst.length; ++j) { const u16 low = src[j * 2]; @@ -184,7 +183,7 @@ namespace rsx::assembler return result; } - FPIR FPIR::from_source(const std::string& asm_) + FPIR FPIR::from_source(std::string_view asm_) { std::vector instructions = fmt::split(asm_, { "\n", ";" }); if (instructions.empty()) @@ -192,13 +191,13 @@ namespace rsx::assembler return {}; } - auto transform_inst = [](const std::string& s) + auto transform_inst = [](std::string_view s) { std::string result; result.reserve(s.size()); bool literal = false; - for (auto& c : s) + for (const auto& c : s) { if (c == ' ') { @@ -235,7 +234,7 @@ namespace rsx::assembler return result; }; - auto decode_instruction = [&](const std::string& inst, std::string& op, std::string& dst, std::vector& sources) + auto decode_instruction = [&](std::string_view inst, std::string& op, std::string& dst, std::vector& sources) { const auto i = transform_inst(inst); if (i.empty()) @@ -259,7 +258,7 @@ namespace rsx::assembler } }; - auto get_ref = [](const std::string& reg) + auto get_ref = [](std::string_view reg) { ensure(reg.length() > 1, "Invalid register specifier"); @@ -291,15 +290,15 @@ namespace rsx::assembler return ref; }; - auto get_constants = [](const std::string& reg) -> std::array + auto get_constants = [](std::string_view reg) -> std::array { float x, y, z, w; - if (sscanf_s(reg.c_str(), "#{%f|%f|%f|%f}", &x, &y, &z, &w) == 4) + if (sscanf_s(reg.data(), "#{%f|%f|%f|%f}", &x, &y, &z, &w) == 4) { return { x, y, z, w }; } - if (sscanf_s(reg.c_str(), "#{%f}", &x) == 1) + if (sscanf_s(reg.data(), "#{%f}", &x) == 1) { return { x, x, x, x }; } @@ -328,35 +327,29 @@ namespace rsx::assembler } }; - auto encode_opcode = [](const std::string& op, Instruction* inst) + auto encode_opcode = [](std::string_view op, Instruction* inst) { OPDEST d0 { .HEX = inst->bytecode[0] }; SRC0 s0 { .HEX = inst->bytecode[1] }; SRC1 s1 { .HEX = inst->bytecode[2] }; -#define SET_OPCODE(encoding) \ - do { \ - inst->opcode = encoding.op; \ - d0.opcode = encoding.op & 0x3F; \ - s1.opcode_hi = (encoding.op > 0x3F)? 1 : 0; \ - s0.exec_if_eq = encoding.exec_if_eq ? 1 : 0; \ - s0.exec_if_gr = encoding.exec_if_gt ? 1 : 0; \ - s0.exec_if_lt = encoding.exec_if_lt ? 1 : 0; \ - d0.set_cond = encoding.set_cond ? 1 : 0; \ - inst->bytecode[0] = d0.HEX; \ - inst->bytecode[1] = s0.HEX; \ - inst->bytecode[2] = s1.HEX; \ - } while (0) - const auto found = s_opcode_lookup.find(op); if (found == s_opcode_lookup.end()) { fmt::throw_exception("Unhandled instruction '%s'", op); } + const auto& encoding = found->second; - SET_OPCODE(found->second); - -#undef SET_OPCODE + inst->opcode = encoding.op; + d0.opcode = encoding.op & 0x3F; + s1.opcode_hi = (encoding.op > 0x3F)? 1 : 0; + s0.exec_if_eq = encoding.exec_if_eq ? 1 : 0; + s0.exec_if_gr = encoding.exec_if_gt ? 1 : 0; + s0.exec_if_lt = encoding.exec_if_lt ? 1 : 0; + d0.set_cond = encoding.set_cond ? 1 : 0; + inst->bytecode[0] = d0.HEX; + inst->bytecode[1] = s0.HEX; + inst->bytecode[2] = s1.HEX; }; std::string op, dst; diff --git a/rpcs3/Emu/RSX/Program/Assembler/FPASM.h b/rpcs3/Emu/RSX/Program/Assembler/FPASM.h index 4f413c21c3..83fc2fb6b1 100644 --- a/rpcs3/Emu/RSX/Program/Assembler/FPASM.h +++ b/rpcs3/Emu/RSX/Program/Assembler/FPASM.h @@ -16,7 +16,7 @@ namespace rsx::assembler const std::vector& instructions() const; std::vector compile() const; - static FPIR from_source(const std::string& asm_); + static FPIR from_source(std::string_view asm_); private: Instruction* load(const RegisterRef& reg, int operand, Instruction* target = nullptr); diff --git a/rpcs3/Emu/RSX/Program/Assembler/Passes/FP/RegisterAnnotationPass.cpp b/rpcs3/Emu/RSX/Program/Assembler/Passes/FP/RegisterAnnotationPass.cpp index 58a589c6f7..4f63f364b6 100644 --- a/rpcs3/Emu/RSX/Program/Assembler/Passes/FP/RegisterAnnotationPass.cpp +++ b/rpcs3/Emu/RSX/Program/Assembler/Passes/FP/RegisterAnnotationPass.cpp @@ -16,9 +16,9 @@ namespace rsx::assembler::FP bool is_delay_slot(const Instruction& instruction) { - OPDEST dst{ .HEX = instruction.bytecode[0] }; - SRC0 src0{ .HEX = instruction.bytecode[1] }; - SRC1 src1{ .HEX = instruction.bytecode[2] }; + const OPDEST dst{ .HEX = instruction.bytecode[0] }; + const SRC0 src0{ .HEX = instruction.bytecode[1] }; + const SRC1 src1{ .HEX = instruction.bytecode[2] }; if (dst.opcode != RSX_FP_OPCODE_MOV || // These slots are always populated with MOV dst.no_dest || // Must have a sink @@ -70,21 +70,14 @@ namespace rsx::assembler::FP if (ref) { - results.push_back(ref); + results.push_back(std::move(ref)); } } // Helper to check a span for 32-bit access auto match_any_32 = [](const std::span lanes) { - for (const auto& c : lanes) - { - if (c == content_dual || c == content_float32) - { - return true; - } - } - return false; + return std::any_of(lanes.begin(), lanes.end(), FN(x == content_dual || x == content_float32)); }; // F32 register processing @@ -115,7 +108,7 @@ namespace rsx::assembler::FP if (ref) { - results.push_back(ref); + results.push_back(std::move(ref)); } } @@ -142,13 +135,13 @@ namespace rsx::assembler::FP continue; } - instruction.srcs.push_back(reg); + instruction.srcs.push_back(std::move(reg)); } RegisterRef dst = get_dst_register(&instruction); if (dst) { - instruction.dsts.push_back(dst); + instruction.dsts.push_back(std::move(dst)); } } } diff --git a/rpcs3/Emu/RSX/Program/Assembler/Passes/FP/RegisterDependencyPass.cpp b/rpcs3/Emu/RSX/Program/Assembler/Passes/FP/RegisterDependencyPass.cpp index 971f485d62..c5b24b35bc 100644 --- a/rpcs3/Emu/RSX/Program/Assembler/Passes/FP/RegisterDependencyPass.cpp +++ b/rpcs3/Emu/RSX/Program/Assembler/Passes/FP/RegisterDependencyPass.cpp @@ -57,7 +57,7 @@ namespace rsx::assembler::FP RegisterRef ref{ .reg{.id = static_cast(index), .f16 = true } }; ref.mask = mask; - result.push_back(ref); + result.push_back(std::move(ref)); } return result; } @@ -94,7 +94,7 @@ namespace rsx::assembler::FP } ref.reg = {.id = static_cast(index), .f16 = false }; - result.push_back(barrier); + result.push_back(std::move(barrier)); } return result; @@ -190,7 +190,7 @@ namespace rsx::assembler::FP Register src_reg{ .id = static_cast(src_reg_id), .f16 = true }; instruction.srcs.push_back({ .reg = src_reg, .mask = 0xF }); instruction.dsts.push_back({ .reg{ .id = reg_id, .f16 = false }, .mask = (1u << ch) }); - result.push_back(instruction); + result.push_back(std::move(instruction)); } return result; @@ -258,7 +258,7 @@ namespace rsx::assembler::FP Register src_reg{ .id = static_cast(src_reg_id), .f16 = true }; instruction.srcs.push_back({ .reg = src_reg, .mask = 0xF }); instruction.dsts.push_back({ .reg{.id = reg.reg.id, .f16 = false }, .mask = dst.write_mask }); - result.push_back(instruction); + result.push_back(std::move(instruction)); } return result; @@ -284,7 +284,7 @@ namespace rsx::assembler::FP for (const auto& barrier : barriers) { auto instructions = build_barrier32(barrier); - result.insert(result.end(), instructions.begin(), instructions.end()); + result.insert(result.end(), std::make_move_iterator(instructions.begin()), std::make_move_iterator(instructions.end())); } return result; @@ -351,10 +351,10 @@ namespace rsx::assembler::FP for (const auto& reg : barrier16_in) { auto barrier = build_barrier16(reg); - instructions.insert(instructions.end(), barrier.begin(), barrier.end()); + instructions.insert(instructions.end(), std::make_move_iterator(barrier.begin()), std::make_move_iterator(barrier.end())); } - it = block->instructions.insert(it, instructions.begin(), instructions.end()); + it = block->instructions.insert(it, std::make_move_iterator(instructions.begin()), std::make_move_iterator(instructions.end())); std::advance(it, instructions.size()); } @@ -367,10 +367,10 @@ namespace rsx::assembler::FP for (const auto& reg : barrier32_in) { auto barrier = build_barrier32(reg); - instructions.insert(instructions.end(), barrier.begin(), barrier.end()); + instructions.insert(instructions.end(), std::make_move_iterator(barrier.begin()), std::make_move_iterator(barrier.end())); } - it = block->instructions.insert(it, instructions.begin(), instructions.end()); + it = block->instructions.insert(it, std::make_move_iterator(instructions.begin()), std::make_move_iterator(instructions.end())); std::advance(it, instructions.size()); } } @@ -431,8 +431,8 @@ namespace rsx::assembler::FP if (!clobbered_lanes.empty()) { - const auto instructions = resolve_dependencies(clobbered_lanes, f16); - target->epilogue.insert(target->epilogue.end(), instructions.begin(), instructions.end()); + auto instructions = resolve_dependencies(clobbered_lanes, f16); + target->epilogue.insert(target->epilogue.end(), std::make_move_iterator(instructions.begin()), std::make_move_iterator(instructions.end())); } if (lanes_to_search.empty())