From 0cde8ab9e8563b4e5d2343da37a4abf4a3b3c41e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 16 Jun 2019 19:21:05 -0400 Subject: [PATCH 1/3] Common/GekkoDisassembler: Make all lookup tables immutable Allows these arrays to be placed within the read-only segment (and enforces the immutability in the code itself). While we're at it, we can make use of std::array here. --- Source/Core/Common/GekkoDisassembler.cpp | 91 ++++++++++++++++-------- 1 file changed, 62 insertions(+), 29 deletions(-) diff --git a/Source/Core/Common/GekkoDisassembler.cpp b/Source/Core/Common/GekkoDisassembler.cpp index ade9b761f7..c1cec303a1 100644 --- a/Source/Core/Common/GekkoDisassembler.cpp +++ b/Source/Core/Common/GekkoDisassembler.cpp @@ -34,12 +34,12 @@ #include "Common/GekkoDisassembler.h" +#include #include #include #include "Common/CommonTypes.h" -#include "Common/StringUtil.h" namespace Common { @@ -86,37 +86,68 @@ namespace Common #define PPCGETIDX2(x) (((x)&PPCIDX2MASK) >> PPCIDX2SH) #define PPCGETSTRM(x) (((x)&PPCSTRM) >> PPCDSH) -static const char* trap_condition[32] = { +constexpr std::array trap_condition{ nullptr, "lgt", "llt", nullptr, "eq", "lge", "lle", nullptr, "gt", nullptr, nullptr, nullptr, "ge", nullptr, nullptr, nullptr, "lt", nullptr, nullptr, nullptr, "le", nullptr, nullptr, nullptr, - "ne", nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr}; + "ne", nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, +}; -static const char* cmpname[4] = {"cmpw", "cmpd", "cmplw", "cmpld"}; +constexpr std::array cmpname{ + "cmpw", + "cmpd", + "cmplw", + "cmpld", +}; -static const char* ps_cmpname[4] = {"ps_cmpu0", "ps_cmpo0", "ps_cmpu1", "ps_cmpo1"}; +constexpr std::array ps_cmpname{ + "ps_cmpu0", + "ps_cmpo0", + "ps_cmpu1", + "ps_cmpo1", +}; -static const char* b_ext[4] = {"", "l", "a", "la"}; +constexpr std::array b_ext{ + "", + "l", + "a", + "la", +}; -static const char* b_condition[8] = {"ge", "le", "ne", "ns", "lt", "gt", "eq", "so"}; +constexpr std::array b_condition{ + "ge", "le", "ne", "ns", "lt", "gt", "eq", "so", +}; -static const char* b_decr[16] = {"nzf", "zf", nullptr, nullptr, "nzt", "zt", nullptr, nullptr, - "nz", "z", nullptr, nullptr, "nz", "z", nullptr, nullptr}; +constexpr std::array b_decr{ + "nzf", "zf", nullptr, nullptr, "nzt", "zt", nullptr, nullptr, + "nz", "z", nullptr, nullptr, "nz", "z", nullptr, nullptr, +}; -static const char* regsel[2] = {"", "r"}; +constexpr std::array regsel{ + "", + "r", +}; -static const char* oesel[2] = {"", "o"}; +constexpr std::array oesel{ + "", + "o", +}; -static const char* rcsel[2] = {"", "."}; +constexpr std::array rcsel{ + "", + ".", +}; -static const char* ldstnames[24] = {"lwz", "lwzu", "lbz", "lbzu", "stw", "stwu", "stb", "stbu", - "lhz", "lhzu", "lha", "lhau", "sth", "sthu", "lmw", "stmw", - "lfs", "lfsu", "lfd", "lfdu", "stfs", "stfsu", "stfd", "stfdu"}; +constexpr std::array ldstnames{ + "lwz", "lwzu", "lbz", "lbzu", "stw", "stwu", "stb", "stbu", "lhz", "lhzu", "lha", "lhau", + "sth", "sthu", "lmw", "stmw", "lfs", "lfsu", "lfd", "lfdu", "stfs", "stfsu", "stfd", "stfdu", +}; -static const char* regnames[32] = {"r0", "sp", "rtoc", "r3", "r4", "r5", "r6", "r7", - "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", - "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23", - "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31"}; +constexpr std::array regnames{ + "r0", "sp", "rtoc", "r3", "r4", "r5", "r6", "r7", "r8", "r9", "r10", + "r11", "r12", "r13", "r14", "r15", "r16", "r17", "r18", "r19", "r20", "r21", + "r22", "r23", "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31", +}; // Initialize static class variables. u32* GekkoDisassembler::m_instr = nullptr; @@ -2328,28 +2359,30 @@ std::string GekkoDisassembler::Disassemble(u32 opcode, u32 current_instruction_a return m_opcode.append("\t").append(m_operands); } -static const char* gprnames[] = { +constexpr std::array gpr_names{ " r0", " r1 (sp)", " r2 (rtoc)", " r3", " r4", " r5", " r6", " r7", " r8", " r9", "r10", "r11", "r12", "r13", "r14", "r15", "r16", "r17", "r18", "r19", "r20", "r21", - "r22", "r23", "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31"}; + "r22", "r23", "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31", +}; const char* GekkoDisassembler::GetGPRName(u32 index) { - if (index < 32) - return gprnames[index]; + if (index < gpr_names.size()) + return gpr_names[index]; return nullptr; } -static const char* fprnames[] = {" f0", " f1", " f2", " f3", " f4", " f5", " f6", " f7", - " f8", " f9", "f10", "f11", "f12", "f13", "f14", "f15", - "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23", - "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31"}; +constexpr std::array fpr_names{ + " f0", " f1", " f2", " f3", " f4", " f5", " f6", " f7", " f8", " f9", "f10", + "f11", "f12", "f13", "f14", "f15", "f16", "f17", "f18", "f19", "f20", "f21", + "f22", "f23", "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", +}; const char* GekkoDisassembler::GetFPRName(u32 index) { - if (index < 32) - return fprnames[index]; + if (index < fpr_names.size()) + return fpr_names[index]; return nullptr; } From d8c3f09c9fcf2f152a4cf6977ab3385092877187 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 16 Jun 2019 19:25:25 -0400 Subject: [PATCH 2/3] Common/GekkoDisassembler: Amend disassembly of operations expecting a character literal Due to the lack of cast here, this will actually print out the ascii value, rather than the character itself, due to promoting to integral values. Instead, we can eliminate the use of character operands and just print the value itself directly, given it's equivalent behavior with less code. --- Source/Core/Common/GekkoDisassembler.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/Common/GekkoDisassembler.cpp b/Source/Core/Common/GekkoDisassembler.cpp index c1cec303a1..2a77d9d985 100644 --- a/Source/Core/Common/GekkoDisassembler.cpp +++ b/Source/Core/Common/GekkoDisassembler.cpp @@ -504,7 +504,7 @@ void GekkoDisassembler::cmpi(u32 in, int uimm) i = (int)PPCGETCRD(in); if (i != 0) { - m_operands += fmt::format("cr{}, ", '0' + i); + m_operands += fmt::format("cr{}, ", i); } m_operands += imm(in, uimm, 2, false); @@ -716,7 +716,7 @@ void GekkoDisassembler::cmp(u32 in) i = (int)PPCGETCRD(in); if (i != 0) - m_operands += fmt::format("cr{},", static_cast('0' + i)); + m_operands += fmt::format("cr{},", i); m_operands += ra_rb(in); } @@ -1220,7 +1220,7 @@ void GekkoDisassembler::ps(u32 inst) int i = (int)PPCGETCRD(inst); if (i != 0) - m_operands += fmt::format("cr{}, ", '0' + i); + m_operands += fmt::format("cr{}, ", i); m_operands += fmt::format("p{}, p{}", FA, FB); return; } From 188234b4cd9b029e3778191432f34c9c6fc279dd Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 16 Jun 2019 19:48:26 -0400 Subject: [PATCH 3/3] Common/GekkoDisassembler: Use std::string_view where applicable Avoids the use of the null pointer to represent an empty string. Instead, we can simply pass an empty string_view instance. Using std::string_view enforces this invariant at the API level. --- Source/Core/Common/GekkoDisassembler.cpp | 38 ++++++++++++------------ Source/Core/Common/GekkoDisassembler.h | 28 ++++++++--------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/Source/Core/Common/GekkoDisassembler.cpp b/Source/Core/Common/GekkoDisassembler.cpp index 2a77d9d985..5f3e959e75 100644 --- a/Source/Core/Common/GekkoDisassembler.cpp +++ b/Source/Core/Common/GekkoDisassembler.cpp @@ -515,7 +515,7 @@ void GekkoDisassembler::cmpi(u32 in, int uimm) } } -void GekkoDisassembler::addi(u32 in, const std::string& ext) +void GekkoDisassembler::addi(u32 in, std::string_view ext) { if ((in & 0x08000000) && !PPCGETA(in)) { @@ -539,7 +539,7 @@ void GekkoDisassembler::addi(u32 in, const std::string& ext) } // Build a branch instr. and return number of chars written to operand. -size_t GekkoDisassembler::branch(u32 in, const char* bname, int aform, int bdisp) +size_t GekkoDisassembler::branch(u32 in, std::string_view bname, int aform, int bdisp) { int bo = (int)PPCGETD(in); int bi = (int)PPCGETA(in); @@ -639,7 +639,7 @@ void GekkoDisassembler::mcrf(u32 in, char c) } } -void GekkoDisassembler::crop(u32 in, const char* n1, const char* n2) +void GekkoDisassembler::crop(u32 in, std::string_view n1, std::string_view n2) { int crd = (int)PPCGETD(in); int cra = (int)PPCGETA(in); @@ -647,8 +647,8 @@ void GekkoDisassembler::crop(u32 in, const char* n1, const char* n2) if ((in & 1) == 0) { - m_opcode = fmt::format("cr{}", (cra == crb && n2) ? n2 : n1); - if (cra == crb && n2) + m_opcode = fmt::format("cr{}", (cra == crb && !n2.empty()) ? n2 : n1); + if (cra == crb && !n2.empty()) m_operands = fmt::format("{}, {}", crd, cra); else m_operands = fmt::format("{}, {}, {}", crd, cra, crb); @@ -659,7 +659,7 @@ void GekkoDisassembler::crop(u32 in, const char* n1, const char* n2) } } -void GekkoDisassembler::nooper(u32 in, const char* name, unsigned char dmode) +void GekkoDisassembler::nooper(u32 in, std::string_view name, unsigned char dmode) { if (in & (PPCDMASK | PPCAMASK | PPCBMASK | 1)) { @@ -672,7 +672,7 @@ void GekkoDisassembler::nooper(u32 in, const char* name, unsigned char dmode) } } -void GekkoDisassembler::rlw(u32 in, const char* name, int i) +void GekkoDisassembler::rlw(u32 in, std::string_view name, int i) { int s = (int)PPCGETD(in); int a = (int)PPCGETA(in); @@ -685,13 +685,13 @@ void GekkoDisassembler::rlw(u32 in, const char* name, int i) bsh, mb, me, HelperRotateMask(bsh, mb, me)); } -void GekkoDisassembler::ori(u32 in, const char* name) +void GekkoDisassembler::ori(u32 in, std::string_view name) { m_opcode = name; m_operands = imm(in, 1, 1, true); } -void GekkoDisassembler::rld(u32 in, const char* name, int i) +void GekkoDisassembler::rld(u32 in, std::string_view name, int i) { int s = (int)PPCGETD(in); int a = (int)PPCGETA(in); @@ -760,8 +760,8 @@ void GekkoDisassembler::trap(u32 in, unsigned char dmode) } // Standard instruction: xxxx rD,rA,rB -void GekkoDisassembler::dab(u32 in, const char* name, int mask, int smode, int chkoe, int chkrc, - unsigned char dmode) +void GekkoDisassembler::dab(u32 in, std::string_view name, int mask, int smode, int chkoe, + int chkrc, unsigned char dmode) { if (chkrc >= 0 && ((in & 1) != (unsigned int)chkrc)) { @@ -782,7 +782,7 @@ void GekkoDisassembler::dab(u32 in, const char* name, int mask, int smode, int c } // Last operand is no register: xxxx rD,rA,NB -void GekkoDisassembler::rrn(u32 in, const char* name, int smode, int chkoe, int chkrc, +void GekkoDisassembler::rrn(u32 in, std::string_view name, int smode, int chkoe, int chkrc, unsigned char dmode) { if (chkrc >= 0 && ((in & 1) != (unsigned int)chkrc)) @@ -943,7 +943,7 @@ void GekkoDisassembler::sradi(u32 in) m_operands = fmt::format("{}, {}, {}", regnames[a], regnames[s], bsh); } -void GekkoDisassembler::ldst(u32 in, const char* name, char reg, unsigned char dmode) +void GekkoDisassembler::ldst(u32 in, std::string_view name, char reg, unsigned char dmode) { int s = (int)PPCGETD(in); int a = (int)PPCGETA(in); @@ -968,7 +968,7 @@ void GekkoDisassembler::ldst(u32 in, const char* name, char reg, unsigned char d } // Standard floating point instruction: xxxx fD,fA,fC,fB -void GekkoDisassembler::fdabc(u32 in, const char* name, int mask, unsigned char dmode) +void GekkoDisassembler::fdabc(u32 in, std::string_view name, int mask, unsigned char dmode) { int err = 0; @@ -1005,7 +1005,7 @@ void GekkoDisassembler::fmr(u32 in) } // Indexed float instruction: xxxx fD,rA,rB -void GekkoDisassembler::fdab(u32 in, const char* name, int mask) +void GekkoDisassembler::fdab(u32 in, std::string_view name, int mask) { m_opcode = name; m_operands = fd_ra_rb(in, mask); @@ -1390,7 +1390,7 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian) break; case 129: - crop(in, "andc", nullptr); // crandc + crop(in, "andc", {}); // crandc break; case 150: @@ -1402,11 +1402,11 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian) break; case 225: - crop(in, "nand", nullptr); // crnand + crop(in, "nand", {}); // crnand break; case 257: - crop(in, "and", nullptr); // crand + crop(in, "and", {}); // crand break; case 289: @@ -1414,7 +1414,7 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian) break; case 417: - crop(in, "orc", nullptr); // crorc + crop(in, "orc", {}); // crorc break; case 449: diff --git a/Source/Core/Common/GekkoDisassembler.h b/Source/Core/Common/GekkoDisassembler.h index a77f1c6989..bc13f4d868 100644 --- a/Source/Core/Common/GekkoDisassembler.h +++ b/Source/Core/Common/GekkoDisassembler.h @@ -34,11 +34,10 @@ #pragma once -#include #include +#include #include "Common/CommonTypes.h" -#include "Common/StringUtil.h" namespace Common { @@ -62,30 +61,31 @@ private: static void trapi(u32 in, unsigned char dmode); static void cmpi(u32 in, int uimm); - static void addi(u32 in, const std::string& ext); - static size_t branch(u32 in, const char* bname, int aform, int bdisp); + static void addi(u32 in, std::string_view ext); + static size_t branch(u32 in, std::string_view bname, int aform, int bdisp); static void bc(u32 in); static void bli(u32 in); static void mcrf(u32 in, char c); - static void crop(u32 in, const char* n1, const char* n2); - static void nooper(u32 in, const char* name, unsigned char dmode); - static void rlw(u32 in, const char* name, int i); - static void ori(u32 in, const char* name); - static void rld(u32 in, const char* name, int i); + static void crop(u32 in, std::string_view n1, std::string_view n2); + static void nooper(u32 in, std::string_view name, unsigned char dmode); + static void rlw(u32 in, std::string_view name, int i); + static void ori(u32 in, std::string_view name); + static void rld(u32 in, std::string_view name, int i); static void cmp(u32 in); static void trap(u32 in, unsigned char dmode); - static void dab(u32 in, const char* name, int mask, int smode, int chkoe, int chkrc, + static void dab(u32 in, std::string_view name, int mask, int smode, int chkoe, int chkrc, + unsigned char dmode); + static void rrn(u32 in, std::string_view name, int smode, int chkoe, int chkrc, unsigned char dmode); - static void rrn(u32 in, const char* name, int smode, int chkoe, int chkrc, unsigned char dmode); static void mtcr(u32 in); static void msr(u32 in, int smode); static void mspr(u32 in, int smode); static void mtb(u32 in); static void sradi(u32 in); - static void ldst(u32 in, const char* name, char reg, unsigned char dmode); - static void fdabc(u32 in, const char* name, int mask, unsigned char dmode); + static void ldst(u32 in, std::string_view name, char reg, unsigned char dmode); + static void fdabc(u32 in, std::string_view name, int mask, unsigned char dmode); static void fmr(u32 in); - static void fdab(u32 in, const char* name, int mask); + static void fdab(u32 in, std::string_view name, int mask); static void fcmp(u32 in, char c); static void mtfsb(u32 in, int n); static void ps(u32 inst);