Debugger: Implement SPU breakpoints

rpcs3/Emu/CPU/CPUThread.cpp

@@ -52,6 +52,7 @@ void fmt_class_string<cpu_flag>::format(std::string& out, u64 arg)
 		case cpu_flag::signal: return "sig";
 		case cpu_flag::memory: return "mem";
 		case cpu_flag::pending: return "pend";
+		case cpu_flag::pending_recheck: return "pend-re";
 		case cpu_flag::dbg_global_pause: return "G-PAUSE";
 		case cpu_flag::dbg_pause: return "PAUSE";
 		case cpu_flag::dbg_step: return "STEP";
@@ -624,6 +625,11 @@ cpu_thread::cpu_thread(u32 id)
 	}
 
 	g_threads_created++;
+
+	if (u32* pc2 = get_pc2())
+	{
+		*pc2 = umax;
+	}
 }
 
 void cpu_thread::cpu_wait(bs_t<cpu_flag> old)
@@ -762,10 +768,18 @@ bool cpu_thread::check_state() noexcept
 				cpu_counter::add(this);
 			}
 
-			if ((state0 & (cpu_flag::pending + cpu_flag::temp)) == cpu_flag::pending)
+			constexpr auto pending_and_temp = (cpu_flag::pending + cpu_flag::temp);
+
+			if ((state0 & pending_and_temp) == cpu_flag::pending)
 			{
 				// Execute pending work
 				cpu_work();
+
+				if ((state1 ^ state) - pending_and_temp)
+				{
+					// Work could have changed flags
+					continue;
+				}
 			}
 
 			if (retval)

rpcs3/Emu/CPU/CPUThread.h

@@ -22,6 +22,7 @@ enum class cpu_flag : u32
 	signal, // Thread received a signal (HLE)
 	memory, // Thread must unlock memory mutex
 	pending, // Thread has postponed work
+	pending_recheck, // Thread needs to recheck if there is pending work before ::pending removal
 
 	dbg_global_pause, // Emulation paused
 	dbg_pause, // Thread paused

rpcs3/Emu/Cell/SPUThread.cpp

@@ -1503,10 +1503,30 @@ void spu_thread::cpu_work()
 
 	const u32 old_iter_count = cpu_work_iteration_count++;
 
-	const auto timeout = +g_cfg.core.mfc_transfers_timeout;
-
 	bool work_left = false;
 
+	if (has_active_local_bps)
+	{
+		if (local_breakpoints[pc / 4])
+		{
+			// Ignore repeatations until a different instruction is issued
+			if (pc != current_bp_pc)
+			{
+				// Breakpoint hit
+				state += cpu_flag::dbg_pause;
+			}
+		}
+
+		current_bp_pc = pc;
+		work_left = true;
+	}
+	else
+	{
+		current_bp_pc = umax;
+	}
+
+	const auto timeout = +g_cfg.core.mfc_transfers_timeout;
+
 	if (u32 shuffle_count = g_cfg.core.mfc_transfers_shuffling)
 	{
 		// If either MFC size exceeds limit or timeout has been reached execute pending MFC commands
@@ -1544,7 +1564,19 @@ void spu_thread::cpu_work()
 
 	if (!work_left)
 	{
-		state -= cpu_flag::pending;
+		// No more pending work
+		state.atomic_op([](bs_t<cpu_flag>& flags)
+		{
+			if (flags & cpu_flag::pending_recheck)
+			{
+				// Do not really remove ::pending because external thread may have pushed more pending work
+				flags -= cpu_flag::pending_recheck;
+			}
+			else
+			{
+				flags -= cpu_flag::pending;
+			}
+		});
 	}
 
 	if (gen_interrupt)

rpcs3/Emu/Cell/SPUThread.h

@@ -843,6 +843,11 @@ public:
 
 	atomic_t<u8> debugger_float_mode = 0;
 
+	// PC-based breakpoint list
+	std::array<atomic_t<bool>, SPU_LS_SIZE / 4> local_breakpoints{};
+	atomic_t<bool> has_active_local_bps = false;
+	u32 current_bp_pc = umax;
+
 	void push_snr(u32 number, u32 value);
 	static void do_dma_transfer(spu_thread* _this, const spu_mfc_cmd& args, u8* ls);
 	bool do_dma_check(const spu_mfc_cmd& args);

rpcs3/rpcs3qt/breakpoint_list.cpp

@@ -3,6 +3,7 @@
 
 #include "Emu/CPU/CPUDisAsm.h"
 #include "Emu/Cell/PPUThread.h"
+#include "Emu/Cell/SPUThread.h"
 
 #include <QMenu>
 #include <QMessageBox>
@@ -11,7 +12,7 @@ constexpr auto qstr = QString::fromStdString;
 
 extern bool is_using_interpreter(u32 id_type);
 
-breakpoint_list::breakpoint_list(QWidget* parent, breakpoint_handler* handler) : QListWidget(parent), m_breakpoint_handler(handler)
+breakpoint_list::breakpoint_list(QWidget* parent, breakpoint_handler* handler) : QListWidget(parent), m_ppu_breakpoint_handler(handler)
 {
 	setEditTriggers(QAbstractItemView::NoEditTriggers);
 	setContextMenuPolicy(Qt::CustomContextMenu);
@@ -42,14 +43,14 @@ void breakpoint_list::ClearBreakpoints()
 	{
 		auto* currentItem = takeItem(0);
 		const u32 loc = currentItem->data(Qt::UserRole).value<u32>();
-		m_breakpoint_handler->RemoveBreakpoint(loc);
+		m_ppu_breakpoint_handler->RemoveBreakpoint(loc);
 		delete currentItem;
 	}
 }
 
 void breakpoint_list::RemoveBreakpoint(u32 addr)
 {
-	m_breakpoint_handler->RemoveBreakpoint(addr);
+	m_ppu_breakpoint_handler->RemoveBreakpoint(addr);
 
 	for (int i = 0; i < count(); i++)
 	{
@@ -67,7 +68,7 @@ void breakpoint_list::RemoveBreakpoint(u32 addr)
 
 bool breakpoint_list::AddBreakpoint(u32 pc)
 {
-	if (!m_breakpoint_handler->AddBreakpoint(pc))
+	if (!m_ppu_breakpoint_handler->AddBreakpoint(pc))
 	{
 		return false;
 	}
@@ -98,10 +99,55 @@ void breakpoint_list::HandleBreakpointRequest(u32 loc)
 		return;
 	}
 
-	if (m_cpu->id_type() != 1)
+	if (!is_using_interpreter(m_cpu->id_type()))
 	{
-		// TODO: SPU breakpoints
+		QMessageBox::warning(this, tr("Interpreters-Only Feature!"), tr("Cannot set breakpoints on non-interpreter decoders."));
-		QMessageBox::warning(this, tr("Unimplemented Breakpoints For Thread Type!"), tr("Cannot set breakpoints on non-PPU thread currently, sorry."));
+		return;
+	}
+
+	switch (m_cpu->id_type())
+	{
+	case 2:
+	{
+		if (loc >= SPU_LS_SIZE || loc % 4)
+		{
+			QMessageBox::warning(this, tr("Invalid Memory For Breakpoints!"), tr("Cannot set breakpoints on non-SPU executable memory!"));
+			return;
+		}
+
+		const auto spu = static_cast<spu_thread*>(m_cpu);
+		auto& list = spu->local_breakpoints;
+
+		if (list[loc / 4].test_and_invert())
+		{
+			if (std::none_of(list.begin(), list.end(), [](auto& val){ return val.load(); }))
+			{
+				spu->has_active_local_bps = false;
+			}
+		}
+		else
+		{
+			if (!spu->has_active_local_bps.exchange(true))
+			{
+				spu->state.atomic_op([](bs_t<cpu_flag>& flags)
+				{
+					if (flags & cpu_flag::pending)
+					{
+						flags += cpu_flag::pending_recheck;
+					}
+					else
+					{
+						flags += cpu_flag::pending;
+					}
+				});
+			}
+		}
+
+		return;
+	}
+	case 1: break;
+	default:
+		QMessageBox::warning(this, tr("Unimplemented Breakpoints For Thread Type!"), tr("Cannot set breakpoints on a thread not an PPU/SPU currently, sorry."));
 		return;
 	}
 
@@ -111,13 +157,7 @@ void breakpoint_list::HandleBreakpointRequest(u32 loc)
 		return;
 	}
 
-	if (!is_using_interpreter(m_cpu->id_type()))
+	if (m_ppu_breakpoint_handler->HasBreakpoint(loc))
-	{
-		QMessageBox::warning(this, tr("Interpreters-Only Feature!"), tr("Cannot set breakpoints on non-interpreter decoders."));
-		return;
-	}
-
-	if (m_breakpoint_handler->HasBreakpoint(loc))
 	{
 		RemoveBreakpoint(loc);
 	}

rpcs3/rpcs3qt/breakpoint_list.h

@@ -30,7 +30,7 @@ private Q_SLOTS:
 	void OnBreakpointListRightClicked(const QPoint &pos);
 	void OnBreakpointListDelete();
 private:
-	breakpoint_handler* m_breakpoint_handler;
+	breakpoint_handler* m_ppu_breakpoint_handler;
 	QMenu* m_context_menu = nullptr;
 	QAction* m_delete_action;
 	cpu_thread* m_cpu = nullptr;

rpcs3/rpcs3qt/debugger_frame.cpp

@@ -80,10 +80,10 @@ debugger_frame::debugger_frame(std::shared_ptr<gui_settings> gui_settings, QWidg
 	QHBoxLayout* hbox_b_main = new QHBoxLayout();
 	hbox_b_main->setContentsMargins(0, 0, 0, 0);
 
-	m_breakpoint_handler = new breakpoint_handler();
+	m_ppu_breakpoint_handler = new breakpoint_handler();
-	m_breakpoint_list = new breakpoint_list(this, m_breakpoint_handler);
+	m_breakpoint_list = new breakpoint_list(this, m_ppu_breakpoint_handler);
 
-	m_debugger_list = new debugger_list(this, m_gui_settings, m_breakpoint_handler);
+	m_debugger_list = new debugger_list(this, m_gui_settings, m_ppu_breakpoint_handler);
 	m_debugger_list->installEventFilter(this);
 
 	m_call_stack_list = new call_stack_list(this);
@@ -823,7 +823,7 @@ void debugger_frame::UpdateUnitList()
 	{
 		if (emu_state == system_state::stopped) return;
 
-		const QVariant var_cpu = QVariant::fromValue<std::pair<cpu_thread*, u32>>(std::make_pair(&cpu, id));
+		const QVariant var_cpu = QVariant::fromValue<data_type>(std::make_pair(&cpu, id));
 
 		// Space at the end is to pad a gap on the right
 		m_choice_units->addItem(qstr((id >> 24 == 0x55 ? "RSX[0x55555555]" : cpu.get_name()) + ' '), var_cpu);
@@ -869,7 +869,7 @@ void debugger_frame::UpdateUnitList()
 
 void debugger_frame::OnSelectUnit()
 {
-	auto [selected, cpu_id] = m_choice_units->currentData().value<std::pair<cpu_thread*, u32>>();
+	auto [selected, cpu_id] = m_choice_units->currentData().value<data_type>();
 
 	if (m_emu_state != system_state::stopped)
 	{
@@ -963,7 +963,7 @@ void debugger_frame::DoUpdate()
 	// Check if we need to disable a step over bp
 	if (const auto cpu0 = get_cpu(); cpu0 && m_last_step_over_breakpoint != umax && cpu0->get_pc() == m_last_step_over_breakpoint)
 	{
-		m_breakpoint_handler->RemoveBreakpoint(m_last_step_over_breakpoint);
+		m_ppu_breakpoint_handler->RemoveBreakpoint(m_last_step_over_breakpoint);
 		m_last_step_over_breakpoint = -1;
 	}
 
@@ -1116,13 +1116,13 @@ void debugger_frame::DoStep(bool step_over)
 
 				// Set breakpoint on next instruction
 				const u32 next_instruction_pc = current_instruction_pc + 4;
-				m_breakpoint_handler->AddBreakpoint(next_instruction_pc);
+				m_ppu_breakpoint_handler->AddBreakpoint(next_instruction_pc);
 
 				// Undefine previous step over breakpoint if it hasnt been already
 				// This can happen when the user steps over a branch that doesn't return to itself
 				if (m_last_step_over_breakpoint != umax)
 				{
-					m_breakpoint_handler->RemoveBreakpoint(next_instruction_pc);
+					m_ppu_breakpoint_handler->RemoveBreakpoint(next_instruction_pc);
 				}
 
 				m_last_step_over_breakpoint = next_instruction_pc;

rpcs3/rpcs3qt/debugger_frame.h

@@ -64,7 +64,7 @@ class debugger_frame : public custom_dock_widget
 	rsx::thread* m_rsx = nullptr;
 
 	breakpoint_list* m_breakpoint_list;
-	breakpoint_handler* m_breakpoint_handler;
+	breakpoint_handler* m_ppu_breakpoint_handler;
 	call_stack_list* m_call_stack_list;
 	instruction_editor_dialog* m_inst_editor = nullptr;
 	register_editor_dialog* m_reg_editor = nullptr;

rpcs3/rpcs3qt/debugger_list.cpp

@@ -23,7 +23,7 @@ constexpr auto qstr = QString::fromStdString;
 debugger_list::debugger_list(QWidget* parent, std::shared_ptr<gui_settings> gui_settings, breakpoint_handler* handler)
 	: QListWidget(parent)
 	, m_gui_settings(std::move(gui_settings))
-	, m_breakpoint_handler(handler)
+	, m_ppu_breakpoint_handler(handler)
 {
 	setWindowTitle(tr("ASM"));
 
@@ -54,9 +54,21 @@ u32 debugger_list::GetCenteredAddress(u32 address) const
 
 void debugger_list::ShowAddress(u32 addr, bool select_addr, bool force)
 {
-	auto IsBreakpoint = [this](u32 pc)
+	const decltype(spu_thread::local_breakpoints)* spu_bps_list;
+
+	if (m_cpu && m_cpu->id_type() == 2)
 	{
-		return m_cpu && m_cpu->id_type() == 1 && m_breakpoint_handler->HasBreakpoint(pc);
+		spu_bps_list = &static_cast<spu_thread*>(m_cpu)->local_breakpoints;
+	}
+
+	auto IsBreakpoint = [&](u32 pc)
+	{
+		switch (m_cpu ? m_cpu->id_type() : 0)
+		{
+		case 1: return m_ppu_breakpoint_handler->HasBreakpoint(pc);
+		case 2: return (*spu_bps_list)[pc / 4].load();
+		default: return false;
+		}
 	};
 
 	bool center_pc = m_gui_settings->GetValue(gui::d_centerPC).toBool();

rpcs3/rpcs3qt/debugger_list.h

@@ -52,7 +52,7 @@ private:
 
 	std::shared_ptr<gui_settings> m_gui_settings;
 
-	breakpoint_handler* m_breakpoint_handler;
+	breakpoint_handler* m_ppu_breakpoint_handler;
 	cpu_thread* m_cpu = nullptr;
 	CPUDisAsm* m_disasm = nullptr;
 	QDialog* m_cmd_detail = nullptr;