From b0b03af50dd17ee7bb9358f368d04c326db139e0 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 13 Nov 2022 16:01:13 -0800 Subject: [PATCH 1/5] Debugger: Keep flag for any breakpoints. Oops, only had these checks for memchecks before. --- Core/Debugger/Breakpoints.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index ad1d37e41d01..9ab4bc490179 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -31,6 +31,7 @@ #include "Core/MIPS/JitCommon/JitCommon.h" #include "Core/CoreTiming.h" +std::atomic anyBreakPoints_(false); std::atomic anyMemChecks_(false); static std::mutex breakPointsMutex_; @@ -160,6 +161,8 @@ size_t CBreakPoints::FindMemCheck(u32 start, u32 end) bool CBreakPoints::IsAddressBreakPoint(u32 addr) { + if (!anyBreakPoints_) + return false; std::lock_guard guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr); return bp != INVALID_BREAKPOINT && breakPoints_[bp].result != BREAK_ACTION_IGNORE; @@ -167,6 +170,8 @@ bool CBreakPoints::IsAddressBreakPoint(u32 addr) bool CBreakPoints::IsAddressBreakPoint(u32 addr, bool* enabled) { + if (!anyBreakPoints_) + return false; std::lock_guard guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr); if (bp == INVALID_BREAKPOINT) return false; @@ -184,6 +189,8 @@ bool CBreakPoints::IsTempBreakPoint(u32 addr) bool CBreakPoints::RangeContainsBreakPoint(u32 addr, u32 size) { + if (!anyBreakPoints_) + return false; std::lock_guard guard(breakPointsMutex_); const u32 end = addr + size; for (const auto &bp : breakPoints_) @@ -207,6 +214,7 @@ void CBreakPoints::AddBreakPoint(u32 addr, bool temp) pt.addr = addr; breakPoints_.push_back(pt); + anyBreakPoints_ = true; guard.unlock(); Update(addr); } @@ -232,6 +240,7 @@ void CBreakPoints::RemoveBreakPoint(u32 addr) if (bp != INVALID_BREAKPOINT) breakPoints_.erase(breakPoints_.begin() + bp); + anyBreakPoints_ = !breakPoints_.empty(); guard.unlock(); Update(addr); } @@ -267,6 +276,8 @@ void CBreakPoints::ChangeBreakPoint(u32 addr, BreakAction result) void CBreakPoints::ClearAllBreakPoints() { + if (!anyBreakPoints_) + return; std::unique_lock guard(breakPointsMutex_); if (!breakPoints_.empty()) { @@ -278,9 +289,9 @@ void CBreakPoints::ClearAllBreakPoints() void CBreakPoints::ClearTemporaryBreakPoints() { - std::unique_lock guard(breakPointsMutex_); - if (breakPoints_.empty()) + if (!anyBreakPoints_) return; + std::unique_lock guard(breakPointsMutex_); bool update = false; for (int i = (int)breakPoints_.size()-1; i >= 0; --i) @@ -342,6 +353,8 @@ void CBreakPoints::ChangeBreakPointLogFormat(u32 addr, const std::string &fmt) { } BreakAction CBreakPoints::ExecBreakPoint(u32 addr) { + if (!anyBreakPoints_) + return BREAK_ACTION_IGNORE; std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr, false); if (bp != INVALID_BREAKPOINT) { From 1662bd3bb80c1c278370993ac5f38f45f7048fde Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 13 Nov 2022 16:03:29 -0800 Subject: [PATCH 2/5] interp: Allow resume from breakpoint. --- Core/MIPS/MIPSTables.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Core/MIPS/MIPSTables.cpp b/Core/MIPS/MIPSTables.cpp index 6a411f42baf8..08deea9e831f 100644 --- a/Core/MIPS/MIPSTables.cpp +++ b/Core/MIPS/MIPSTables.cpp @@ -976,8 +976,7 @@ int MIPSInterpret_RunUntil(u64 globalTicks) //2: check for breakpoint (VERY SLOW) #if defined(_DEBUG) - if (CBreakPoints::IsAddressBreakPoint(curMips->pc)) - { + if (CBreakPoints::IsAddressBreakPoint(curMips->pc) && CBreakPoints::CheckSkipFirst() != curMips->pc) { auto cond = CBreakPoints::GetBreakPointCondition(currentMIPS->pc); if (!cond || cond->Evaluate()) { From 76cf4dbf1296dc9e2704939187dad1f823487950 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 13 Nov 2022 16:48:16 -0800 Subject: [PATCH 3/5] interp: Allow breakpoints in release mode. --- Core/Debugger/Breakpoints.cpp | 10 ++- Core/Debugger/Breakpoints.h | 1 + Core/MIPS/MIPSTables.cpp | 133 +++++++++++++++++++++------------- 3 files changed, 88 insertions(+), 56 deletions(-) diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index 9ab4bc490179..5ce977ec45d7 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -646,10 +646,12 @@ const std::vector CBreakPoints::GetBreakpoints() return breakPoints_; } -bool CBreakPoints::HasMemChecks() -{ - std::lock_guard guard(memCheckMutex_); - return !memChecks_.empty(); +bool CBreakPoints::HasBreakPoints() { + return anyBreakPoints_; +} + +bool CBreakPoints::HasMemChecks() { + return anyMemChecks_; } void CBreakPoints::Update(u32 addr) { diff --git a/Core/Debugger/Breakpoints.h b/Core/Debugger/Breakpoints.h index c2a7d81b6a46..9f1a5751188f 100644 --- a/Core/Debugger/Breakpoints.h +++ b/Core/Debugger/Breakpoints.h @@ -167,6 +167,7 @@ class CBreakPoints static const std::vector GetMemChecks(); static const std::vector GetBreakpoints(); + static bool HasBreakPoints(); static bool HasMemChecks(); static void Update(u32 addr = 0); diff --git a/Core/MIPS/MIPSTables.cpp b/Core/MIPS/MIPSTables.cpp index 08deea9e831f..48182a37b512 100644 --- a/Core/MIPS/MIPSTables.cpp +++ b/Core/MIPS/MIPSTables.cpp @@ -938,8 +938,7 @@ void MIPSDisAsm(MIPSOpcode op, u32 pc, char *out, bool tabsToSpaces) { } } -void MIPSInterpret(MIPSOpcode op) { - const MIPSInstruction *instr = MIPSGetInstruction(op); +static inline void Interpret(const MIPSInstruction *instr, MIPSOpcode op) { if (instr && instr->interpret) { instr->interpret(op); } else { @@ -947,69 +946,99 @@ void MIPSInterpret(MIPSOpcode op) { // Try to disassemble it char disasm[256]; MIPSDisAsm(op, currentMIPS->pc, disasm); - _dbg_assert_msg_( 0, "%s", disasm); + _dbg_assert_msg_(0, "%s", disasm); currentMIPS->pc += 4; } } +inline int GetInstructionCycleEstimate(const MIPSInstruction *instr) { + if (instr) + return instr->flags.cycles; + return 1; +} + +void MIPSInterpret(MIPSOpcode op) { + const MIPSInstruction *instr = MIPSGetInstruction(op); + Interpret(instr, op); +} + #define _RS ((op>>21) & 0x1F) #define _RT ((op>>16) & 0x1F) #define _RD ((op>>11) & 0x1F) #define R(i) (curMips->r[i]) - -int MIPSInterpret_RunUntil(u64 globalTicks) -{ +static inline void RunUntilFast() { MIPSState *curMips = currentMIPS; - while (coreState == CORE_RUNNING) - { - CoreTiming::Advance(); + // NEVER stop in a delay slot! + while (curMips->downcount >= 0 && coreState == CORE_RUNNING) { + do { + // Replacements and similar are processed here, intentionally. + MIPSOpcode op = MIPSOpcode(Memory::Read_U32(curMips->pc)); + + bool wasInDelaySlot = curMips->inDelaySlot; + const MIPSInstruction *instr = MIPSGetInstruction(op); + Interpret(instr, op); + curMips->downcount -= GetInstructionCycleEstimate(instr); + + // The reason we have to check this is the delay slot hack in Int_Syscall. + if (curMips->inDelaySlot && wasInDelaySlot) { + curMips->pc = curMips->nextPC; + curMips->inDelaySlot = false; + } + } while (curMips->inDelaySlot); + } +} - // NEVER stop in a delay slot! - while (curMips->downcount >= 0 && coreState == CORE_RUNNING) - { - // int cycles = 0; - { - again: - MIPSOpcode op = MIPSOpcode(Memory::Read_U32(curMips->pc)); - //MIPSOpcode op = Memory::Read_Opcode_JIT(mipsr4k.pc); - - //2: check for breakpoint (VERY SLOW) -#if defined(_DEBUG) - if (CBreakPoints::IsAddressBreakPoint(curMips->pc) && CBreakPoints::CheckSkipFirst() != curMips->pc) { - auto cond = CBreakPoints::GetBreakPointCondition(currentMIPS->pc); - if (!cond || cond->Evaluate()) - { - Core_EnableStepping(true, "cpu.breakpoint", curMips->pc); - if (CBreakPoints::IsTempBreakPoint(curMips->pc)) - CBreakPoints::RemoveBreakPoint(curMips->pc); - break; - } - } -#endif - - bool wasInDelaySlot = curMips->inDelaySlot; - MIPSInterpret(op); - curMips->downcount -= MIPSGetInstructionCycleEstimate(op); - - if (curMips->inDelaySlot) - { - // The reason we have to check this is the delay slot hack in Int_Syscall. - if (wasInDelaySlot) - { - curMips->pc = curMips->nextPC; - curMips->inDelaySlot = false; - continue; - } - goto again; +static void RunUntilWithChecks(u64 globalTicks) { + MIPSState *curMips = currentMIPS; + // NEVER stop in a delay slot! + while (curMips->downcount >= 0 && coreState == CORE_RUNNING) { + do { + // Replacements and similar are processed here, intentionally. + MIPSOpcode op = MIPSOpcode(Memory::Read_U32(curMips->pc)); + + // Check for breakpoint + if (CBreakPoints::IsAddressBreakPoint(curMips->pc) && CBreakPoints::CheckSkipFirst() != curMips->pc) { + auto cond = CBreakPoints::GetBreakPointCondition(currentMIPS->pc); + if (!cond || cond->Evaluate()) { + Core_EnableStepping(true, "cpu.breakpoint", curMips->pc); + if (CBreakPoints::IsTempBreakPoint(curMips->pc)) + CBreakPoints::RemoveBreakPoint(curMips->pc); + break; } } - if (CoreTiming::GetTicks() > globalTicks) - { - // DEBUG_LOG(CPU, "Hit the max ticks, bailing 1 : %llu, %llu", globalTicks, CoreTiming::GetTicks()); - return 1; + bool wasInDelaySlot = curMips->inDelaySlot; + const MIPSInstruction *instr = MIPSGetInstruction(op); + Interpret(instr, op); + curMips->downcount -= GetInstructionCycleEstimate(instr); + + // The reason we have to check this is the delay slot hack in Int_Syscall. + if (curMips->inDelaySlot && wasInDelaySlot) { + curMips->pc = curMips->nextPC; + curMips->inDelaySlot = false; } + } while (curMips->inDelaySlot); + + if (CoreTiming::GetTicks() > globalTicks) + return; + } +} + +int MIPSInterpret_RunUntil(u64 globalTicks) { + MIPSState *curMips = currentMIPS; + while (coreState == CORE_RUNNING) { + CoreTiming::Advance(); + + uint64_t ticksLeft = globalTicks - CoreTiming::GetTicks(); + if (CBreakPoints::HasBreakPoints() || CBreakPoints::HasMemChecks() || ticksLeft <= curMips->downcount) + RunUntilWithChecks(globalTicks); + else + RunUntilFast(); + + if (CoreTiming::GetTicks() > globalTicks) { + // DEBUG_LOG(CPU, "Hit the max ticks, bailing 1 : %llu, %llu", globalTicks, CoreTiming::GetTicks()); + return 1; } } @@ -1048,8 +1077,8 @@ MIPSInterpretFunc MIPSGetInterpretFunc(MIPSOpcode op) // TODO: Do something that makes sense here. int MIPSGetInstructionCycleEstimate(MIPSOpcode op) { - MIPSInfo info = MIPSGetInfo(op); - return info.cycles; + const MIPSInstruction *instr = MIPSGetInstruction(op); + return GetInstructionCycleEstimate(instr); } const char *MIPSDisasmAt(u32 compilerPC) { From f9da9e6b609add7d66a81d4376a5ea1fcf69cbe5 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 13 Nov 2022 17:38:53 -0800 Subject: [PATCH 4/5] interp: Centralize memory size handling. --- Core/Debugger/DisassemblyManager.cpp | 18 ++------------ Core/MIPS/MIPSAnalyst.cpp | 36 ++-------------------------- Core/MIPS/MIPSInt.h | 2 -- Core/MIPS/MIPSIntVFPU.h | 1 - Core/MIPS/MIPSTables.cpp | 21 ++++++++++++++++ Core/MIPS/MIPSTables.h | 1 + 6 files changed, 26 insertions(+), 53 deletions(-) diff --git a/Core/Debugger/DisassemblyManager.cpp b/Core/Debugger/DisassemblyManager.cpp index ab0e10aa9bf5..b57e52610d6b 100644 --- a/Core/Debugger/DisassemblyManager.cpp +++ b/Core/Debugger/DisassemblyManager.cpp @@ -709,22 +709,8 @@ void DisassemblyFunction::load() case 0x2B: // sw macro = new DisassemblyMacro(opAddress); - int dataSize; - switch (nextInfo & MEMTYPE_MASK) { - case MEMTYPE_BYTE: - dataSize = 1; - break; - case MEMTYPE_HWORD: - dataSize = 2; - break; - case MEMTYPE_WORD: - case MEMTYPE_FLOAT: - dataSize = 4; - break; - case MEMTYPE_VQUAD: - dataSize = 16; - break; - default: + int dataSize = MIPSGetMemoryAccessSize(next); + if (dataSize == 0) { delete macro; return; } diff --git a/Core/MIPS/MIPSAnalyst.cpp b/Core/MIPS/MIPSAnalyst.cpp index 353d7605d393..cdeaa6a169b8 100644 --- a/Core/MIPS/MIPSAnalyst.cpp +++ b/Core/MIPS/MIPSAnalyst.cpp @@ -611,25 +611,7 @@ namespace MIPSAnalyst { int OpMemoryAccessSize(u32 pc) { const auto op = Memory::Read_Instruction(pc, true); - MIPSInfo info = MIPSGetInfo(op); - if ((info & (IN_MEM | OUT_MEM)) == 0) { - return 0; - } - - // TODO: Verify lwl/lwr/etc.? - switch (info & MEMTYPE_MASK) { - case MEMTYPE_BYTE: - return 1; - case MEMTYPE_HWORD: - return 2; - case MEMTYPE_WORD: - case MEMTYPE_FLOAT: - return 4; - case MEMTYPE_VQUAD: - return 16; - } - - return 0; + return MIPSGetMemoryAccessSize(op); } bool IsOpMemoryWrite(u32 pc) { @@ -1553,21 +1535,7 @@ namespace MIPSAnalyst { // lw, sh, ... if (!IsSyscall(op) && (opInfo & (IN_MEM | OUT_MEM)) != 0) { info.isDataAccess = true; - switch (opInfo & MEMTYPE_MASK) { - case MEMTYPE_BYTE: - info.dataSize = 1; - break; - case MEMTYPE_HWORD: - info.dataSize = 2; - break; - case MEMTYPE_WORD: - case MEMTYPE_FLOAT: - info.dataSize = 4; - break; - - case MEMTYPE_VQUAD: - info.dataSize = 16; - } + info.dataSize = MIPSGetMemoryAccessSize(op); u32 rs = cpu->GetRegValue(0, (int)MIPS_GET_RS(op)); s16 imm16 = op & 0xFFFF; diff --git a/Core/MIPS/MIPSInt.h b/Core/MIPS/MIPSInt.h index cd52fc409b9d..039a5f597fc5 100644 --- a/Core/MIPS/MIPSInt.h +++ b/Core/MIPS/MIPSInt.h @@ -23,8 +23,6 @@ int MIPS_SingleStep(); namespace MIPSInt { - void Int_Unknown(MIPSOpcode op); - void Int_Unimpl(MIPSOpcode op); void Int_Syscall(MIPSOpcode op); void Int_mxc1(MIPSOpcode op); diff --git a/Core/MIPS/MIPSIntVFPU.h b/Core/MIPS/MIPSIntVFPU.h index 1d0a1ba1f1a8..fdd7954aa3f6 100644 --- a/Core/MIPS/MIPSIntVFPU.h +++ b/Core/MIPS/MIPSIntVFPU.h @@ -26,7 +26,6 @@ namespace MIPSInt void Int_SV(MIPSOpcode op); void Int_SVQ(MIPSOpcode op); void Int_Mftv(MIPSOpcode op); - void Int_MatrixSet(MIPSOpcode op); void Int_VecDo3(MIPSOpcode op); void Int_Vcst(MIPSOpcode op); void Int_VMatrixInit(MIPSOpcode op); diff --git a/Core/MIPS/MIPSTables.cpp b/Core/MIPS/MIPSTables.cpp index 48182a37b512..5a2065074bf4 100644 --- a/Core/MIPS/MIPSTables.cpp +++ b/Core/MIPS/MIPSTables.cpp @@ -1081,6 +1081,27 @@ int MIPSGetInstructionCycleEstimate(MIPSOpcode op) return GetInstructionCycleEstimate(instr); } +int MIPSGetMemoryAccessSize(MIPSOpcode op) { + MIPSInfo info = MIPSGetInfo(op); + if ((info & (IN_MEM | OUT_MEM)) == 0) { + return 0; + } + + switch (info & MEMTYPE_MASK) { + case MEMTYPE_BYTE: + return 1; + case MEMTYPE_HWORD: + return 2; + case MEMTYPE_WORD: + case MEMTYPE_FLOAT: + return 4; + case MEMTYPE_VQUAD: + return 16; + } + + return 0; +} + const char *MIPSDisasmAt(u32 compilerPC) { static char temp[256]; MIPSDisAsm(Memory::Read_Instruction(compilerPC), 0, temp); diff --git a/Core/MIPS/MIPSTables.h b/Core/MIPS/MIPSTables.h index fd12c43a9642..484eab35ea35 100644 --- a/Core/MIPS/MIPSTables.h +++ b/Core/MIPS/MIPSTables.h @@ -130,5 +130,6 @@ int MIPSInterpret_RunUntil(u64 globalTicks); MIPSInterpretFunc MIPSGetInterpretFunc(MIPSOpcode op); int MIPSGetInstructionCycleEstimate(MIPSOpcode op); +int MIPSGetMemoryAccessSize(MIPSOpcode op); const char *MIPSGetName(MIPSOpcode op); const char *MIPSDisasmAt(u32 compilerPC); From 0f79afa172db43421785448143505f9d4546111e Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 13 Nov 2022 17:44:44 -0800 Subject: [PATCH 5/5] interp: Support memory breakpoints too. --- Core/Debugger/Breakpoints.h | 2 +- Core/MIPS/MIPSTables.cpp | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Core/Debugger/Breakpoints.h b/Core/Debugger/Breakpoints.h index 9f1a5751188f..b60858e79e24 100644 --- a/Core/Debugger/Breakpoints.h +++ b/Core/Debugger/Breakpoints.h @@ -115,7 +115,7 @@ struct MemCheck { // BreakPoints cannot overlap, only one is allowed per address. // MemChecks can overlap, as long as their ends are different. -// WARNING: MemChecks are not used in the interpreter or HLE currently. +// WARNING: MemChecks are not always tracked in HLE currently. class CBreakPoints { public: diff --git a/Core/MIPS/MIPSTables.cpp b/Core/MIPS/MIPSTables.cpp index 5a2065074bf4..996a54daf793 100644 --- a/Core/MIPS/MIPSTables.cpp +++ b/Core/MIPS/MIPSTables.cpp @@ -992,13 +992,16 @@ static inline void RunUntilFast() { static void RunUntilWithChecks(u64 globalTicks) { MIPSState *curMips = currentMIPS; // NEVER stop in a delay slot! + bool hasBPs = CBreakPoints::HasBreakPoints(); + bool hasMCs = CBreakPoints::HasMemChecks(); while (curMips->downcount >= 0 && coreState == CORE_RUNNING) { do { // Replacements and similar are processed here, intentionally. MIPSOpcode op = MIPSOpcode(Memory::Read_U32(curMips->pc)); + const MIPSInstruction *instr = MIPSGetInstruction(op); // Check for breakpoint - if (CBreakPoints::IsAddressBreakPoint(curMips->pc) && CBreakPoints::CheckSkipFirst() != curMips->pc) { + if (hasBPs && CBreakPoints::IsAddressBreakPoint(curMips->pc) && CBreakPoints::CheckSkipFirst() != curMips->pc) { auto cond = CBreakPoints::GetBreakPointCondition(currentMIPS->pc); if (!cond || cond->Evaluate()) { Core_EnableStepping(true, "cpu.breakpoint", curMips->pc); @@ -1007,9 +1010,23 @@ static void RunUntilWithChecks(u64 globalTicks) { break; } } + if (hasMCs && (instr->flags & (IN_MEM | OUT_MEM)) != 0 && CBreakPoints::CheckSkipFirst() != curMips->pc && instr->interpret != &Int_Syscall) { + // This is common for all IN_MEM/OUT_MEM funcs. + int offset = (instr->flags & IS_VFPU) != 0 ? SignExtend16ToS32(op & 0xFFFC) : SignExtend16ToS32(op); + u32 addr = (R(_RS) + offset) & 0xFFFFFFFC; + int sz = MIPSGetMemoryAccessSize(op); + + if ((instr->flags & IN_MEM) != 0) + CBreakPoints::ExecMemCheck(addr, false, sz, curMips->pc, "interpret"); + if ((instr->flags & OUT_MEM) != 0) + CBreakPoints::ExecMemCheck(addr, true, sz, curMips->pc, "interpret"); + + // If it tripped, bail without running. + if (coreState == CORE_STEPPING) + break; + } bool wasInDelaySlot = curMips->inDelaySlot; - const MIPSInstruction *instr = MIPSGetInstruction(op); Interpret(instr, op); curMips->downcount -= GetInstructionCycleEstimate(instr);