From 09f3c446b8519e5448e6370cf846cb43f1dba809 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Wed, 5 Apr 2023 06:36:40 -0700 Subject: [PATCH 1/5] Debugger: Improve perf with write-only memchecks. --- Core/Debugger/Breakpoints.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index dae383c7ed..edfa7bf20d 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -616,7 +616,7 @@ u32 CBreakPoints::CheckSkipFirst() const std::vector CBreakPoints::GetMemCheckRanges(bool write) { std::lock_guard guard(memCheckMutex_); - std::vector ranges = memChecks_; + std::vector ranges; for (const auto &check : memChecks_) { if (!(check.cond & MEMCHECK_READ) && !write) continue; @@ -628,6 +628,7 @@ const std::vector CBreakPoints::GetMemCheckRanges(bool write) { copy.start ^= 0x40000000; if (copy.end != 0) copy.end ^= 0x40000000; + ranges.push_back(check); ranges.push_back(copy); } From 1d3f262eda1bd3f1c642c0ec76150c88a50536a1 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Wed, 5 Apr 2023 06:57:42 -0700 Subject: [PATCH 2/5] Debugger: Trigger mem breakpoints for mirrors. --- Core/Debugger/Breakpoints.cpp | 44 +++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index edfa7bf20d..08ea9dbe02 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -489,9 +489,10 @@ bool CBreakPoints::GetMemCheck(u32 start, u32 end, MemCheck *check) { return false; } -static inline u32 NotCached(u32 val) -{ - // Remove the cached part of the address. +static inline u32 NotCached(u32 val) { + // Remove the cached part of the address as well as any mirror. + if ((val & 0x3F800000) == 0x04000000) + return val & ~0x40600000; return val & ~0x40000000; } @@ -614,6 +615,26 @@ u32 CBreakPoints::CheckSkipFirst() return 0; } +static MemCheck NotCached(MemCheck mc) { + // Toggle the cached part of the address. + mc.start ^= 0x40000000; + if (mc.end != 0) + mc.end ^= 0x40000000; + return mc; +} + +static MemCheck VRAMMirror(uint8_t mirror, MemCheck mc) { + mc.start &= ~0x00600000; + mc.start += 0x00200000 * mirror; + if (mc.end != 0) { + mc.end &= ~0x00600000; + mc.end += 0x00200000 * mirror; + if (mc.end < mc.start) + mc.end += 0x00200000; + } + return mc; +} + const std::vector CBreakPoints::GetMemCheckRanges(bool write) { std::lock_guard guard(memCheckMutex_); std::vector ranges; @@ -623,13 +644,16 @@ const std::vector CBreakPoints::GetMemCheckRanges(bool write) { if (!(check.cond & MEMCHECK_WRITE) && write) continue; - MemCheck copy = check; - // Toggle the cached part of the address. - copy.start ^= 0x40000000; - if (copy.end != 0) - copy.end ^= 0x40000000; - ranges.push_back(check); - ranges.push_back(copy); + if (Memory::IsVRAMAddress(check.start) && (check.end == 0 || Memory::IsVRAMAddress(check.end))) { + for (uint8_t mirror = 0; mirror < 4; ++mirror) { + MemCheck copy = VRAMMirror(mirror, check); + ranges.push_back(copy); + ranges.push_back(NotCached(copy)); + } + } else { + ranges.push_back(check); + ranges.push_back(NotCached(check)); + } } return ranges; From e73474e590b97edc6f61c999ff77cd74e0b29fc1 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Wed, 5 Apr 2023 17:13:00 -0700 Subject: [PATCH 3/5] Debugger: Cache memcheck ranges. Rebuilding these can be a bit slow each time, speed up jit compilation by caching them. --- Core/Debugger/Breakpoints.cpp | 39 +++++++++++++++++++++++++---------- Core/Debugger/Breakpoints.h | 3 +++ 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index 08ea9dbe02..14f0230fd1 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -41,6 +41,8 @@ u64 CBreakPoints::breakSkipFirstTicks_ = 0; static std::mutex memCheckMutex_; std::vector CBreakPoints::memChecks_; std::vector CBreakPoints::cleanupMemChecks_; +std::vector CBreakPoints::memCheckRangesRead_; +std::vector CBreakPoints::memCheckRangesWrite_; void MemCheck::Log(u32 addr, bool write, int size, u32 pc, const char *reason) { if (result & BREAK_ACTION_LOG) { @@ -635,28 +637,40 @@ static MemCheck VRAMMirror(uint8_t mirror, MemCheck mc) { return mc; } -const std::vector CBreakPoints::GetMemCheckRanges(bool write) { +void CBreakPoints::UpdateCachedMemCheckRanges() { std::lock_guard guard(memCheckMutex_); - std::vector ranges; + memCheckRangesRead_.clear(); + memCheckRangesWrite_.clear(); + + auto add = [&](bool read, bool write, const MemCheck &mc) { + if (read) + memCheckRangesRead_.push_back(mc); + if (write) + memCheckRangesWrite_.push_back(mc); + }; + for (const auto &check : memChecks_) { - if (!(check.cond & MEMCHECK_READ) && !write) - continue; - if (!(check.cond & MEMCHECK_WRITE) && write) - continue; + bool read = (check.cond & MEMCHECK_READ) != 0; + bool write = (check.cond & MEMCHECK_WRITE) != 0; if (Memory::IsVRAMAddress(check.start) && (check.end == 0 || Memory::IsVRAMAddress(check.end))) { for (uint8_t mirror = 0; mirror < 4; ++mirror) { MemCheck copy = VRAMMirror(mirror, check); - ranges.push_back(copy); - ranges.push_back(NotCached(copy)); + add(read, write, copy); + add(read, write, NotCached(copy)); } } else { - ranges.push_back(check); - ranges.push_back(NotCached(check)); + add(read, write, check); + add(read, write, NotCached(check)); } } +} - return ranges; +const std::vector CBreakPoints::GetMemCheckRanges(bool write) { + std::lock_guard guard(memCheckMutex_); + if (write) + return memCheckRangesWrite_; + return memCheckRangesRead_; } const std::vector CBreakPoints::GetMemChecks() @@ -698,6 +712,9 @@ void CBreakPoints::Update(u32 addr) { Core_EnableStepping(false); } + if (anyMemChecks_) + UpdateCachedMemCheckRanges(); + // Redraw in order to show the breakpoint. System_Notify(SystemNotification::DISASSEMBLY); } diff --git a/Core/Debugger/Breakpoints.h b/Core/Debugger/Breakpoints.h index b60858e79e..6c3fd91ee9 100644 --- a/Core/Debugger/Breakpoints.h +++ b/Core/Debugger/Breakpoints.h @@ -180,6 +180,7 @@ private: // Finds exactly, not using a range check. static size_t FindMemCheck(u32 start, u32 end); static MemCheck *GetMemCheckLocked(u32 address, int size); + static void UpdateCachedMemCheckRanges(); static std::vector breakPoints_; static u32 breakSkipFirstAt_; @@ -187,6 +188,8 @@ private: static std::vector memChecks_; static std::vector cleanupMemChecks_; + static std::vector memCheckRangesRead_; + static std::vector memCheckRangesWrite_; }; From 142707ccf3d2e9ea44cf870d8cea4f3bf2f59224 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Wed, 5 Apr 2023 17:14:51 -0700 Subject: [PATCH 4/5] x86jit: Reduce memory breakpoint codegen. Was generating a lot of code and, in some games, burning through the jit cache causing constant recompilation with just a few breakpoints. --- Core/MIPS/x86/JitSafeMem.cpp | 58 +++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/Core/MIPS/x86/JitSafeMem.cpp b/Core/MIPS/x86/JitSafeMem.cpp index 8a6e8a6788..e4d3b3e8b3 100644 --- a/Core/MIPS/x86/JitSafeMem.cpp +++ b/Core/MIPS/x86/JitSafeMem.cpp @@ -403,42 +403,42 @@ void JitSafeMem::MemCheckImm(MemoryOpType type) { } } -void JitSafeMem::MemCheckAsm(MemoryOpType type) -{ +void JitSafeMem::MemCheckAsm(MemoryOpType type) { const auto memchecks = CBreakPoints::GetMemCheckRanges(type == MEM_WRITE); bool possible = !memchecks.empty(); - for (auto it = memchecks.begin(), end = memchecks.end(); it != end; ++it) - { - FixupBranch skipNext, skipNextRange; - if (it->end != 0) - { + std::vector hitChecks; + for (auto it = memchecks.begin(), end = memchecks.end(); it != end; ++it) { + if (it->end != 0) { jit_->CMP(32, R(xaddr_), Imm32(it->start - offset_ - size_)); - skipNext = jit_->J_CC(CC_BE); - jit_->CMP(32, R(xaddr_), Imm32(it->end - offset_)); - skipNextRange = jit_->J_CC(CC_AE); - } - else - { - jit_->CMP(32, R(xaddr_), Imm32(it->start - offset_)); - skipNext = jit_->J_CC(CC_NE); - } + FixupBranch skipNext = jit_->J_CC(CC_BE); - // Keep the stack 16-byte aligned, just PUSH/POP 4 times. - for (int i = 0; i < 4; ++i) - jit_->PUSH(xaddr_); + jit_->CMP(32, R(xaddr_), Imm32(it->end - offset_)); + hitChecks.push_back(jit_->J_CC(CC_B, true)); + + jit_->SetJumpTarget(skipNext); + } else { + jit_->CMP(32, R(xaddr_), Imm32(it->start - offset_)); + hitChecks.push_back(jit_->J_CC(CC_E, true)); + } + } + + if (possible) { + FixupBranch noHits = jit_->J(true); + + // Okay, now land any hit here. + for (auto &fixup : hitChecks) + jit_->SetJumpTarget(fixup); + hitChecks.clear(); + + jit_->PUSH(xaddr_); + // Keep the stack 16-byte aligned. + jit_->SUB(PTRBITS, R(SP), Imm32(16 - PTRBITS / 8)); jit_->MOV(32, MIPSSTATE_VAR(pc), Imm32(jit_->GetCompilerPC())); jit_->ADD(32, R(xaddr_), Imm32(offset_)); jit_->CallProtectedFunction(&JitMemCheck, R(xaddr_), size_, type == MEM_WRITE ? 1 : 0); - for (int i = 0; i < 4; ++i) - jit_->POP(xaddr_); + jit_->ADD(PTRBITS, R(SP), Imm32(16 - PTRBITS / 8)); + jit_->POP(xaddr_); - jit_->SetJumpTarget(skipNext); - if (it->end != 0) - jit_->SetJumpTarget(skipNextRange); - } - - if (possible) - { // CORE_RUNNING is <= CORE_NEXTFRAME. if (jit_->RipAccessible((const void *)&coreState)) { jit_->CMP(32, M(&coreState), Imm32(CORE_NEXTFRAME)); // rip accessible @@ -451,6 +451,8 @@ void JitSafeMem::MemCheckAsm(MemoryOpType type) } skipChecks_.push_back(jit_->J_CC(CC_G, true)); jit_->js.afterOp |= JitState::AFTER_CORE_STATE | JitState::AFTER_REWIND_PC_BAD_STATE | JitState::AFTER_MEMCHECK_CLEANUP; + + jit_->SetJumpTarget(noHits); } } From 6df939034a99c7e891097a187a9cb7f277c20b70 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Wed, 5 Apr 2023 17:16:51 -0700 Subject: [PATCH 5/5] Core: Cleanup some sign extensions for clarity. --- Core/MIPS/ARM/ArmCompFPU.cpp | 2 +- Core/MIPS/ARM/ArmCompLoadStore.cpp | 2 +- Core/MIPS/ARM/ArmJit.cpp | 2 +- Core/MIPS/ARM64/Arm64CompFPU.cpp | 4 +--- Core/MIPS/ARM64/Arm64CompLoadStore.cpp | 4 ++-- Core/MIPS/ARM64/Arm64Jit.cpp | 2 +- Core/MIPS/MIPSAnalyst.cpp | 2 +- Core/MIPS/MIPSDis.cpp | 2 +- Core/MIPS/MIPSInt.cpp | 2 +- 9 files changed, 10 insertions(+), 12 deletions(-) diff --git a/Core/MIPS/ARM/ArmCompFPU.cpp b/Core/MIPS/ARM/ArmCompFPU.cpp index f2cc37e3b1..112bae8679 100644 --- a/Core/MIPS/ARM/ArmCompFPU.cpp +++ b/Core/MIPS/ARM/ArmCompFPU.cpp @@ -95,7 +95,7 @@ void ArmJit::Comp_FPULS(MIPSOpcode op) CONDITIONAL_DISABLE(LSU_FPU); CheckMemoryBreakpoint(); - s32 offset = (s16)(op & 0xFFFF); + s32 offset = SignExtend16ToS32(op & 0xFFFF); int ft = _FT; MIPSGPReg rs = _RS; // u32 addr = R(rs) + offset; diff --git a/Core/MIPS/ARM/ArmCompLoadStore.cpp b/Core/MIPS/ARM/ArmCompLoadStore.cpp index 667c0ece44..1c62d7f986 100644 --- a/Core/MIPS/ARM/ArmCompLoadStore.cpp +++ b/Core/MIPS/ARM/ArmCompLoadStore.cpp @@ -113,7 +113,7 @@ namespace MIPSComp void ArmJit::Comp_ITypeMemLR(MIPSOpcode op, bool load) { CONDITIONAL_DISABLE(LSU); CheckMemoryBreakpoint(); - int offset = (signed short)(op & 0xFFFF); + int offset = SignExtend16ToS32(op & 0xFFFF); MIPSGPReg rt = _RT; MIPSGPReg rs = _RS; int o = op >> 26; diff --git a/Core/MIPS/ARM/ArmJit.cpp b/Core/MIPS/ARM/ArmJit.cpp index 238ce2e94d..898310fc68 100644 --- a/Core/MIPS/ARM/ArmJit.cpp +++ b/Core/MIPS/ARM/ArmJit.cpp @@ -91,7 +91,7 @@ static u32 JitMemCheck(u32 pc) { // Note: pc may be the delay slot. const auto op = Memory::Read_Instruction(pc, true); - s32 offset = (s16)(op & 0xFFFF); + s32 offset = SignExtend16ToS32(op & 0xFFFF); if (MIPSGetInfo(op) & IS_VFPU) offset &= 0xFFFC; u32 addr = currentMIPS->r[MIPS_GET_RS(op)] + offset; diff --git a/Core/MIPS/ARM64/Arm64CompFPU.cpp b/Core/MIPS/ARM64/Arm64CompFPU.cpp index efe6887f23..c1cd969582 100644 --- a/Core/MIPS/ARM64/Arm64CompFPU.cpp +++ b/Core/MIPS/ARM64/Arm64CompFPU.cpp @@ -83,9 +83,7 @@ void Arm64Jit::Comp_FPULS(MIPSOpcode op) CONDITIONAL_DISABLE(LSU_FPU); CheckMemoryBreakpoint(); - // Surprisingly, these work fine alraedy. - - s32 offset = (s16)(op & 0xFFFF); + s32 offset = SignExtend16ToS32(op & 0xFFFF); int ft = _FT; MIPSGPReg rs = _RS; // u32 addr = R(rs) + offset; diff --git a/Core/MIPS/ARM64/Arm64CompLoadStore.cpp b/Core/MIPS/ARM64/Arm64CompLoadStore.cpp index 92b241d17d..08e7d541a6 100644 --- a/Core/MIPS/ARM64/Arm64CompLoadStore.cpp +++ b/Core/MIPS/ARM64/Arm64CompLoadStore.cpp @@ -115,7 +115,7 @@ namespace MIPSComp { void Arm64Jit::Comp_ITypeMemLR(MIPSOpcode op, bool load) { CONDITIONAL_DISABLE(LSU); CheckMemoryBreakpoint(); - int offset = (signed short)(op & 0xFFFF); + int offset = SignExtend16ToS32(op & 0xFFFF); MIPSGPReg rt = _RT; MIPSGPReg rs = _RS; int o = op >> 26; @@ -267,7 +267,7 @@ namespace MIPSComp { CONDITIONAL_DISABLE(LSU); CheckMemoryBreakpoint(); - int offset = (signed short)(op & 0xFFFF); + int offset = SignExtend16ToS32(op & 0xFFFF); bool load = false; MIPSGPReg rt = _RT; MIPSGPReg rs = _RS; diff --git a/Core/MIPS/ARM64/Arm64Jit.cpp b/Core/MIPS/ARM64/Arm64Jit.cpp index 71415569e5..ec11bd3f99 100644 --- a/Core/MIPS/ARM64/Arm64Jit.cpp +++ b/Core/MIPS/ARM64/Arm64Jit.cpp @@ -81,7 +81,7 @@ static u32 JitMemCheck(u32 pc) { // Note: pc may be the delay slot. const auto op = Memory::Read_Instruction(pc, true); - s32 offset = (s16)(op & 0xFFFF); + s32 offset = SignExtend16ToS32(op & 0xFFFF); if (MIPSGetInfo(op) & IS_VFPU) offset &= 0xFFFC; u32 addr = currentMIPS->r[MIPS_GET_RS(op)] + offset; diff --git a/Core/MIPS/MIPSAnalyst.cpp b/Core/MIPS/MIPSAnalyst.cpp index f4fb504a89..2e682b9b9f 100644 --- a/Core/MIPS/MIPSAnalyst.cpp +++ b/Core/MIPS/MIPSAnalyst.cpp @@ -1449,7 +1449,7 @@ skip: case 0x08: // addi case 0x09: // addiu info.hasRelevantAddress = true; - info.relevantAddress = cpu->GetRegValue(0,MIPS_GET_RS(op))+((s16)(op & 0xFFFF)); + info.relevantAddress = cpu->GetRegValue(0, MIPS_GET_RS(op)) + SignExtend16ToS32(op & 0xFFFF); break; } diff --git a/Core/MIPS/MIPSDis.cpp b/Core/MIPS/MIPSDis.cpp index 22d1e05b21..d62d67dd34 100644 --- a/Core/MIPS/MIPSDis.cpp +++ b/Core/MIPS/MIPSDis.cpp @@ -64,7 +64,7 @@ namespace MIPSDis void Dis_Cache(MIPSOpcode op, char *out) { - int imm = (s16)(op & 0xFFFF); + int imm = SignExtend16ToS32(op & 0xFFFF); int rs = _RS; int func = (op >> 16) & 0x1F; sprintf(out, "%s\tfunc=%i, %s(%s)", MIPSGetName(op), func, RN(rs), SignedHex(imm)); diff --git a/Core/MIPS/MIPSInt.cpp b/Core/MIPS/MIPSInt.cpp index fd326d84a4..95c341c988 100644 --- a/Core/MIPS/MIPSInt.cpp +++ b/Core/MIPS/MIPSInt.cpp @@ -94,7 +94,7 @@ namespace MIPSInt { void Int_Cache(MIPSOpcode op) { - int imm = (s16)(op & 0xFFFF); + int imm = SignExtend16ToS32(op & 0xFFFF); int rs = _RS; uint32_t addr = R(rs) + imm; int func = (op >> 16) & 0x1F;