diff --git a/Core/Core.cpp b/Core/Core.cpp index f229a04249..1ab6180894 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -322,6 +322,11 @@ void Core_ProcessStepping(MIPSDebugInterface *cpu) { // Free-threaded (hm, possibly except tracing). void Core_Break(const char *reason, u32 relatedAddress) { + if (coreState != CORE_RUNNING_CPU) { + ERROR_LOG(Log::CPU, "Core_Break ony works in the CORE_RUNNING_CPU state"); + return; + } + // Stop the tracer { std::lock_guard lock(g_stepMutex); diff --git a/Core/Core.h b/Core/Core.h index dbfa0376ea..4b9cb2abd9 100644 --- a/Core/Core.h +++ b/Core/Core.h @@ -46,7 +46,8 @@ enum class CPUStepType { // Async, called from gui void Core_Break(const char *reason, u32 relatedAddress = 0); -// void Core_Step(CPUStepType type); // CPUStepType::None not allowed + +// Resumes execution. Works both when stepping the CPU and the GE. void Core_Resume(); // This should be called externally. diff --git a/Core/Debugger/DisassemblyManager.cpp b/Core/Debugger/DisassemblyManager.cpp index 1532d434b0..b497532da1 100644 --- a/Core/Debugger/DisassemblyManager.cpp +++ b/Core/Debugger/DisassemblyManager.cpp @@ -39,8 +39,7 @@ std::recursive_mutex DisassemblyManager::entriesLock_; DebugInterface* DisassemblyManager::cpu; int DisassemblyManager::maxParamChars = 29; -bool isInInterval(u32 start, u32 size, u32 value) -{ +bool isInInterval(u32 start, u32 size, u32 value) { return start <= value && value <= (start+size-1); } @@ -196,7 +195,6 @@ void DisassemblyManager::analyze(u32 address, u32 size = 1024) if (!PSP_IsInited()) return; - auto memLock = Memory::Lock(); std::lock_guard guard(entriesLock_); auto it = findDisassemblyEntry(entries, address, false); if (it != entries.end()) @@ -285,12 +283,9 @@ std::vector DisassemblyManager::getBranchLines(u32 start, u32 size) void DisassemblyManager::getLine(u32 address, bool insertSymbols, DisassemblyLineInfo &dest, DebugInterface *cpuDebug) { - // This is here really to avoid lock ordering issues. - auto memLock = Memory::Lock(); std::lock_guard guard(entriesLock_); auto it = findDisassemblyEntry(entries,address,false); - if (it == entries.end()) - { + if (it == entries.end()) { analyze(address); it = findDisassemblyEntry(entries,address,false); } @@ -319,7 +314,6 @@ void DisassemblyManager::getLine(u32 address, bool insertSymbols, DisassemblyLin u32 DisassemblyManager::getStartAddress(u32 address) { - auto memLock = Memory::Lock(); std::lock_guard guard(entriesLock_); auto it = findDisassemblyEntry(entries,address,false); if (it == entries.end()) @@ -337,15 +331,13 @@ u32 DisassemblyManager::getStartAddress(u32 address) u32 DisassemblyManager::getNthPreviousAddress(u32 address, int n) { - auto memLock = Memory::Lock(); std::lock_guard guard(entriesLock_); while (Memory::IsValidAddress(address)) { auto it = findDisassemblyEntry(entries,address,false); if (it == entries.end()) break; - while (it != entries.end()) - { + while (it != entries.end()) { DisassemblyEntry* entry = it->second; int oldLineNum = entry->getLineNum(address,true); if (n <= oldLineNum) @@ -366,7 +358,6 @@ u32 DisassemblyManager::getNthPreviousAddress(u32 address, int n) u32 DisassemblyManager::getNthNextAddress(u32 address, int n) { - auto memLock = Memory::Lock(); std::lock_guard guard(entriesLock_); while (Memory::IsValidAddress(address)) { @@ -401,7 +392,6 @@ DisassemblyManager::~DisassemblyManager() { void DisassemblyManager::clear() { - auto memLock = Memory::Lock(); std::lock_guard guard(entriesLock_); for (auto it = entries.begin(); it != entries.end(); it++) { @@ -412,7 +402,6 @@ void DisassemblyManager::clear() DisassemblyFunction::DisassemblyFunction(u32 _address, u32 _size): address(_address), size(_size) { - auto memLock = Memory::Lock(); if (!PSP_IsInited()) return; @@ -426,7 +415,6 @@ DisassemblyFunction::~DisassemblyFunction() { void DisassemblyFunction::recheck() { - auto memLock = Memory::Lock(); if (!PSP_IsInited()) return; @@ -888,7 +876,6 @@ bool DisassemblyMacro::disassemble(u32 address, DisassemblyLineInfo &dest, bool DisassemblyData::DisassemblyData(u32 _address, u32 _size, DataType _type): address(_address), size(_size), type(_type) { - auto memLock = Memory::Lock(); if (!PSP_IsInited()) return; @@ -898,7 +885,6 @@ DisassemblyData::DisassemblyData(u32 _address, u32 _size, DataType _type): addre void DisassemblyData::recheck() { - auto memLock = Memory::Lock(); if (!PSP_IsInited()) return; diff --git a/GPU/Common/GPUDebugInterface.h b/GPU/Common/GPUDebugInterface.h index f862689b84..39423d63ba 100644 --- a/GPU/Common/GPUDebugInterface.h +++ b/GPU/Common/GPUDebugInterface.h @@ -209,7 +209,7 @@ public: return DisassembleOp(pc, Memory::Read_U32(pc)); } virtual GPUDebugOp DisassembleOp(u32 pc, u32 op) = 0; - virtual std::vector DissassembleOpRange(u32 startpc, u32 endpc) = 0; + virtual std::vector DisassembleOpRange(u32 startpc, u32 endpc) = 0; // Enter/exit stepping mode. Mainly for better debug stats on time taken. virtual void NotifySteppingEnter() = 0; diff --git a/GPU/Common/TextureCacheCommon.cpp b/GPU/Common/TextureCacheCommon.cpp index 8172f59b4e..956dd2558a 100644 --- a/GPU/Common/TextureCacheCommon.cpp +++ b/GPU/Common/TextureCacheCommon.cpp @@ -3101,7 +3101,7 @@ void TextureCacheCommon::DrawImGuiDebug(uint64_t &selectedTextureId) const { ImGui::Image(texId, ImVec2(128, 128)); } - if (!secondCache_.size()) { + if (!secondCache_.empty()) { ImGui::Text("Secondary Cache (%d): TODO", (int)secondCache_.size()); // TODO } diff --git a/GPU/Debugger/Debugger.cpp b/GPU/Debugger/Debugger.cpp index 8ed4917552..c89c0b1c7c 100644 --- a/GPU/Debugger/Debugger.cpp +++ b/GPU/Debugger/Debugger.cpp @@ -39,6 +39,7 @@ static int thisFlipNum = 0; bool g_drawNotified = false; static double lastStepTime = -1.0; +static uint32_t g_skipPcOnce = 0; static std::vector> restrictPrimRanges; static std::string restrictPrimRule; @@ -97,17 +98,6 @@ void SetBreakCount(int c, bool relative) { } } -static bool IsBreakpoint(u32 pc, u32 op) { - if (breakNext == BreakNext::OP) { - return true; - } else if (breakNext == BreakNext::COUNT) { - return primsThisFrame == breakAtCount; - } else if (hasBreakpoints) { - return GPUBreakpoints::IsBreakpoint(pc, op); - } - return false; -} - NotifyResult NotifyCommand(u32 pc) { if (!active) { _dbg_assert_(false); @@ -143,7 +133,23 @@ NotifyResult NotifyCommand(u32 pc) { } } - if (IsBreakpoint(pc, op)) { + bool isBreakpoint = false; + if (breakNext == BreakNext::OP) { + isBreakpoint = true; + } else if (breakNext == BreakNext::COUNT) { + isBreakpoint = primsThisFrame == breakAtCount; + } else if (hasBreakpoints) { + isBreakpoint = GPUBreakpoints::IsBreakpoint(pc, op); + } + + if (isBreakpoint && pc == g_skipPcOnce) { + INFO_LOG(Log::G3D, "Skipping break at %08x (last break was here)", g_skipPcOnce); + g_skipPcOnce = 0; + return process ? NotifyResult::Execute : NotifyResult::Skip; + } + g_skipPcOnce = 0; + + if (isBreakpoint) { GPUBreakpoints::ClearTempBreakpoints(); if (coreState == CORE_POWERDOWN || !gpuDebug) { @@ -158,6 +164,8 @@ NotifyResult NotifyCommand(u32 pc) { } else { NOTICE_LOG(Log::G3D, "Waiting at %08x, %s", pc, info.desc.c_str()); } + + g_skipPcOnce = pc; return NotifyResult::Break; // new. caller will call GPUStepping::EnterStepping(). } diff --git a/GPU/Debugger/Playback.cpp b/GPU/Debugger/Playback.cpp index 42f062d1a5..41a14dc43e 100644 --- a/GPU/Debugger/Playback.cpp +++ b/GPU/Debugger/Playback.cpp @@ -338,7 +338,7 @@ void DumpExecute::SyncStall() { gpu->UpdateStall(execListID, execListPos, &runList); if (runList) { DLResult result = gpu->ProcessDLQueue(); - _dbg_assert_(result == DLResult::Done || result == DLResult::Pause); + _dbg_assert_(result == DLResult::Done || result == DLResult::Stall); } s64 listTicks = gpu->GetListTicks(execListID); if (listTicks != -1) { diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 16b060f8ce..65a9e735f8 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -1637,7 +1637,7 @@ GPUDebugOp GPUCommon::DisassembleOp(u32 pc, u32 op) { return info; } -std::vector GPUCommon::DissassembleOpRange(u32 startpc, u32 endpc) { +std::vector GPUCommon::DisassembleOpRange(u32 startpc, u32 endpc) { char buffer[1024]; std::vector result; GPUDebugOp info; @@ -1995,12 +1995,32 @@ bool GPUCommon::DescribeCodePtr(const u8 *ptr, std::string &name) { } void GPUCommon::DrawImGuiDebugger() { + // Proof of concept + if (ImGui::Button("Run")) { + Core_Resume(); + } + ImGui::SameLine(); + if (ImGui::Button("Next Tex")) { + GPUDebug::SetBreakNext(GPUDebug::BreakNext::TEX); + } + ImGui::SameLine(); + if (ImGui::Button("Next Prim")) { + GPUDebug::SetBreakNext(GPUDebug::BreakNext::PRIM); + } + ImGui::SameLine(); + if (ImGui::Button("Single step")) { + GPUDebug::SetBreakNext(GPUDebug::BreakNext::OP); + } + // First, let's list any active display lists. ImGui::Text("Next list ID: %d", nextListID); for (auto index : dlQueue) { const auto &list = dls[index]; - ImGui::Text("List %d", list.id); - ImGui::Text("pc: %08x (start: %08x)", list.pc, list.startpc); - ImGui::Text("bbox: %d", (int)list.bboxResult); + char title[64]; + snprintf(title, sizeof(title), "List %d", list.id); + if (ImGui::CollapsingHeader(title, ImGuiTreeNodeFlags_DefaultOpen)) { + ImGui::Text("PC: %08x (start: %08x)", list.pc, list.startpc); + ImGui::Text("BBOX result: %d", (int)list.bboxResult); + } } } diff --git a/GPU/GPUCommon.h b/GPU/GPUCommon.h index ad96b8cf41..0d3aaf857e 100644 --- a/GPU/GPUCommon.h +++ b/GPU/GPUCommon.h @@ -141,12 +141,6 @@ namespace Draw { class DrawContext; } -enum class DLResult { - Done, - Error, - Pause, // used for stepping, breakpoints -}; - enum DrawType { DRAW_UNKNOWN, DRAW_PRIM, @@ -355,7 +349,7 @@ public: void ResetListState(int listID, DisplayListState state) override; GPUDebugOp DisassembleOp(u32 pc, u32 op) override; - std::vector DissassembleOpRange(u32 startpc, u32 endpc) override; + std::vector DisassembleOpRange(u32 startpc, u32 endpc) override; void NotifySteppingEnter() override; void NotifySteppingExit() override; diff --git a/GPU/GPUDefinitions.h b/GPU/GPUDefinitions.h index d2ec4ff4a7..d7f1a56aa6 100644 --- a/GPU/GPUDefinitions.h +++ b/GPU/GPUDefinitions.h @@ -17,6 +17,11 @@ #pragma once +// X11, sigh. +#ifdef None +#undef None +#endif + enum DisplayListStatus { // The list has been completed PSP_GE_LIST_COMPLETED = 0, diff --git a/UI/ImDebugger/ImDebugger.cpp b/UI/ImDebugger/ImDebugger.cpp index e6e377b9e8..98b7ed623f 100644 --- a/UI/ImDebugger/ImDebugger.cpp +++ b/UI/ImDebugger/ImDebugger.cpp @@ -132,6 +132,7 @@ static const char *ThreadStatusToString(u32 status) { } void DrawThreadView(ImConfig &cfg) { + ImGui::SetNextWindowSize(ImVec2(420, 300), ImGuiCond_FirstUseEver); if (!ImGui::Begin("Threads", &cfg.threadsOpen)) { ImGui::End(); return; @@ -213,6 +214,7 @@ static void RecurseFileSystem(IFileSystem *fs, std::string path) { } static void DrawFilesystemBrowser(ImConfig &cfg) { + ImGui::SetNextWindowSize(ImVec2(420, 500), ImGuiCond_FirstUseEver); if (!ImGui::Begin("File System", &cfg.filesystemBrowserOpen)) { ImGui::End(); return; @@ -738,7 +740,6 @@ ImDebugger::~ImDebugger() { cfg_.SaveConfig(ConfigPath()); } - void ImDebugger::Frame(MIPSDebugInterface *mipsDebug, GPUDebugInterface *gpuDebug) { // Snapshot the coreState to avoid inconsistency. const CoreState coreState = ::coreState; @@ -753,15 +754,17 @@ void ImDebugger::Frame(MIPSDebugInterface *mipsDebug, GPUDebugInterface *gpuDebu if (ImGui::BeginMainMenuBar()) { if (ImGui::BeginMenu("Debug")) { - if (coreState == CoreState::CORE_STEPPING_CPU) { + switch (coreState) { + case CoreState::CORE_STEPPING_CPU: if (ImGui::MenuItem("Run")) { Core_Resume(); } - // used to have the step commands here, but they belong in the disassembly window. - } else { + break; + case CoreState::CORE_RUNNING_CPU: if (ImGui::MenuItem("Break")) { Core_Break("Menu:Break"); } + break; } ImGui::Separator(); ImGui::MenuItem("Ignore bad memory accesses", nullptr, &g_Config.bIgnoreBadMemAccess); @@ -821,7 +824,7 @@ void ImDebugger::Frame(MIPSDebugInterface *mipsDebug, GPUDebugInterface *gpuDebu ImGui::EndMenu(); } if (ImGui::BeginMenu("Graphics")) { - ImGui::MenuItem("Ge Debugger", nullptr, &cfg_.geDebuggerOpen); + ImGui::MenuItem("GE Debugger", nullptr, &cfg_.geDebuggerOpen); ImGui::MenuItem("Display Output", nullptr, &cfg_.displayOpen); ImGui::MenuItem("Textures", nullptr, &cfg_.texturesOpen); ImGui::MenuItem("Framebuffers", nullptr, &cfg_.framebuffersOpen); @@ -858,7 +861,7 @@ void ImDebugger::Frame(MIPSDebugInterface *mipsDebug, GPUDebugInterface *gpuDebu } if (cfg_.disasmOpen) { - disasm_.Draw(mipsDebug, &cfg_.disasmOpen, coreState); + disasm_.Draw(mipsDebug, cfg_, coreState); } if (cfg_.regsOpen) { @@ -926,14 +929,14 @@ void ImDebugger::Frame(MIPSDebugInterface *mipsDebug, GPUDebugInterface *gpuDebu } } -void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, bool *open, CoreState coreState) { +void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, CoreState coreState) { char title[256]; snprintf(title, sizeof(title), "%s - Disassembly", "Allegrex MIPS"); disasmView_.setDebugger(mipsDebug); ImGui::SetNextWindowSize(ImVec2(520, 600), ImGuiCond_FirstUseEver); - if (!ImGui::Begin(title, open, ImGuiWindowFlags_NoNavInputs)) { + if (!ImGui::Begin(title, &cfg.disasmOpen, ImGuiWindowFlags_NoNavInputs)) { ImGui::End(); return; } @@ -951,7 +954,12 @@ void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, bool *open, CoreState c } if (coreState == CORE_STEPPING_GE || coreState == CORE_RUNNING_GE) { - ImGui::Text("!!! Currently stepping the Ge. See that window (when implemented)"); + ImGui::Text("!!! Currently stepping the GE"); + ImGui::SameLine(); + if (ImGui::SmallButton("Open Ge debugger")) { + cfg.geDebuggerOpen = true; + ImGui::SetWindowFocus("GE Debugger"); + } } ImGui::BeginDisabled(coreState != CORE_STEPPING_CPU); diff --git a/UI/ImDebugger/ImDebugger.h b/UI/ImDebugger/ImDebugger.h index 9c774d72ec..08f4016be3 100644 --- a/UI/ImDebugger/ImDebugger.h +++ b/UI/ImDebugger/ImDebugger.h @@ -26,11 +26,12 @@ class MIPSDebugInterface; class GPUDebugInterface; +struct ImConfig; // Corresponds to the CDisasm dialog class ImDisasmWindow { public: - void Draw(MIPSDebugInterface *mipsDebug, bool *open, CoreState coreState); + void Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, CoreState coreState); ImDisasmView &View() { return disasmView_; } diff --git a/UI/ImDebugger/ImDisasmView.cpp b/UI/ImDebugger/ImDisasmView.cpp index 03bd0cd5e1..a3cd4f051a 100644 --- a/UI/ImDebugger/ImDisasmView.cpp +++ b/UI/ImDebugger/ImDisasmView.cpp @@ -102,7 +102,6 @@ static std::string trimString(std::string input) { void ImDisasmView::assembleOpcode(u32 address, const std::string &defaultText) { /* - auto memLock = Memory::Lock(); if (!Core_IsStepping()) { MessageBox(wnd, L"Cannot change code while the core is running!", L"Error", MB_OK); return; @@ -315,7 +314,6 @@ void ImDisasmView::drawArguments(ImDrawList *drawList, Rect rc, const Disassembl } void ImDisasmView::Draw(ImDrawList *drawList) { - auto memLock = Memory::Lock(); if (!debugger->isAlive()) { return; } @@ -922,7 +920,6 @@ void ImDisasmView::PopupMenu() { } void ImDisasmView::updateStatusBarText() { - auto memLock = Memory::Lock(); if (!PSP_IsInited()) return; @@ -1047,8 +1044,6 @@ void ImDisasmView::SearchNext(bool forward) { return; } - auto memLock = Memory::Lock(); - // Note: Search will replace matchAddress_ with the current address. u32 searchAddress = manager.getNthNextAddress(matchAddress_, 1); @@ -1110,7 +1105,6 @@ void ImDisasmView::SearchNext(bool forward) { } std::string ImDisasmView::disassembleRange(u32 start, u32 size) { - auto memLock = Memory::Lock(); std::string result; // gather all branch targets without labels diff --git a/UI/ImDebugger/ImGe.cpp b/UI/ImDebugger/ImGe.cpp index d686cfc1fd..4534dc650e 100644 --- a/UI/ImDebugger/ImGe.cpp +++ b/UI/ImDebugger/ImGe.cpp @@ -9,6 +9,7 @@ #include "Core/HW/Display.h" void DrawFramebuffersWindow(ImConfig &cfg, FramebufferManagerCommon *framebufferManager) { + ImGui::SetNextWindowSize(ImVec2(520, 600), ImGuiCond_FirstUseEver); if (!ImGui::Begin("Framebuffers", &cfg.framebuffersOpen)) { ImGui::End(); return; @@ -20,6 +21,7 @@ void DrawFramebuffersWindow(ImConfig &cfg, FramebufferManagerCommon *framebuffer } void DrawTexturesWindow(ImConfig &cfg, TextureCacheCommon *textureCache) { + ImGui::SetNextWindowSize(ImVec2(520, 600), ImGuiCond_FirstUseEver); if (!ImGui::Begin("Textures", &cfg.texturesOpen)) { ImGui::End(); return; @@ -31,6 +33,7 @@ void DrawTexturesWindow(ImConfig &cfg, TextureCacheCommon *textureCache) { } void DrawDisplayWindow(ImConfig &cfg, FramebufferManagerCommon *framebufferManager) { + ImGui::SetNextWindowSize(ImVec2(520, 600), ImGuiCond_FirstUseEver); if (!ImGui::Begin("Display", &cfg.displayOpen)) { ImGui::End(); return; @@ -59,6 +62,7 @@ void DrawDisplayWindow(ImConfig &cfg, FramebufferManagerCommon *framebufferManag // Note: This is not exclusively graphics. void DrawDebugStatsWindow(ImConfig &cfg) { + ImGui::SetNextWindowSize(ImVec2(300, 500), ImGuiCond_FirstUseEver); if (!ImGui::Begin("Debug Stats", &cfg.debugStatsOpen)) { ImGui::End(); return; @@ -71,7 +75,8 @@ void DrawDebugStatsWindow(ImConfig &cfg) { // Stub void DrawGeDebuggerWindow(ImConfig &cfg) { - if (!ImGui::Begin("Debug Stats", &cfg.geDebuggerOpen)) { + ImGui::SetNextWindowSize(ImVec2(520, 600), ImGuiCond_FirstUseEver); + if (!ImGui::Begin("GE Debugger", &cfg.geDebuggerOpen)) { ImGui::End(); return; } diff --git a/Windows/Debugger/Debugger_Disasm.cpp b/Windows/Debugger/Debugger_Disasm.cpp index 8569d6bca7..46224da487 100644 --- a/Windows/Debugger/Debugger_Disasm.cpp +++ b/Windows/Debugger/Debugger_Disasm.cpp @@ -407,7 +407,6 @@ BOOL CDisasm::DlgProc(UINT message, WPARAM wParam, LPARAM lParam) { // If the current PC is on a breakpoint, the user doesn't want to do nothing. breakpoints_->SetSkipFirst(currentMIPS->pc); - Core_Resume(); } } diff --git a/Windows/GEDebugger/CtrlDisplayListView.cpp b/Windows/GEDebugger/CtrlDisplayListView.cpp index a55e757c2d..113c7f297a 100644 --- a/Windows/GEDebugger/CtrlDisplayListView.cpp +++ b/Windows/GEDebugger/CtrlDisplayListView.cpp @@ -180,7 +180,7 @@ void CtrlDisplayListView::onPaint(WPARAM wParam, LPARAM lParam) HICON breakPoint = (HICON)LoadIcon(GetModuleHandle(0),(LPCWSTR)IDI_STOP); - auto disasm = gpuDebug->DissassembleOpRange(windowStart, windowStart + (visibleRows + 2) * instructionSize); + auto disasm = gpuDebug->DisassembleOpRange(windowStart, windowStart + (visibleRows + 2) * instructionSize); for (int i = 0; i < visibleRows+2; i++) {