Dolphin debugger enhancements

Source/Core/Common/BreakPoints.cpp

@@ -113,6 +113,19 @@ void BreakPoints::Clear()
 	m_BreakPoints.clear();
 }
 
+void BreakPoints::ClearAllTemporary()
+{
+	for (const TBreakPoint& bp : m_BreakPoints)
+	{
+		if (bp.bTemporary)
+		{
+			if (jit)
+				jit->GetBlockCache()->InvalidateICache(bp.iAddress, 4, true);
+			Remove(bp.iAddress);
+		}
+	}
+}
+
 MemChecks::TMemChecksStr MemChecks::GetStrings() const
 {
 	TMemChecksStr mcs;
@@ -204,3 +217,87 @@ void TMemCheck::Action(DebugInterface *debug_interface, u32 iValue, u32 addr, bo
 			debug_interface->BreakNow();
 	}
 }
+
+
+const bool Watches::IsAddressWatch(u32 _iAddress)
+{
+	for (const TWatch& bp : m_Watches)
+		if (bp.iAddress == _iAddress)
+			return true;
+
+	return false;
+}
+
+Watches::TWatchesStr Watches::GetStrings() const
+{
+	TWatchesStr bps;
+	for (const TWatch& bp : m_Watches)
+	{
+		std::stringstream ss;
+		ss << std::hex << bp.iAddress << " " << bp.name;
+		bps.push_back(ss.str());
+	}
+
+	return bps;
+}
+
+void Watches::AddFromStrings(const TWatchesStr& bpstrs)
+{
+	for (const std::string& bpstr : bpstrs)
+	{
+		TWatch bp;
+		std::stringstream ss;
+		ss << std::hex << bpstr;
+		ss >> bp.iAddress;
+		ss >> std::ws;
+		getline(ss, bp.name);
+		Add(bp);
+	}
+}
+
+void Watches::Add(const TWatch& bp)
+{
+	if (!IsAddressWatch(bp.iAddress))
+	{
+		m_Watches.push_back(bp);
+	}
+}
+
+void Watches::Add(u32 em_address)
+{
+	if (!IsAddressWatch(em_address)) // only add new addresses
+	{
+		TWatch pt; // breakpoint settings
+		pt.bOn = true;
+		pt.iAddress = em_address;
+
+		m_Watches.push_back(pt);
+	}
+}
+
+void Watches::Update(int count, u32 em_address)
+{
+	m_Watches.at(count).iAddress = em_address;
+}
+
+void Watches::UpdateName(int count, const std::string name)
+{
+	m_Watches.at(count).name = name;
+}
+
+void Watches::Remove(u32 em_address)
+{
+	for (auto i = m_Watches.begin(); i != m_Watches.end(); ++i)
+	{
+		if (i->iAddress == em_address)
+		{
+			m_Watches.erase(i);
+			return;
+		}
+	}
+}
+
+void Watches::Clear()
+{
+	m_Watches.clear();
+}

Source/Core/Common/BreakPoints.h

@@ -44,6 +44,13 @@ struct TMemCheck
 	            bool write, int size, u32 pc);
 };
 
+struct TWatch
+{
+	std::string name = "";
+	u32  iAddress;
+	bool bOn;
+};
+
 // Code breakpoints.
 class BreakPoints
 {
@@ -67,6 +74,7 @@ class BreakPoints
 	// Remove Breakpoint
 	void Remove(u32 _iAddress);
 	void Clear();
+	void ClearAllTemporary();
 
 	void DeleteByAddress(u32 _Address);
 
@@ -98,3 +106,33 @@ class MemChecks
 
 	void Clear() { m_MemChecks.clear(); }
 };
+
+class Watches
+{
+public:
+	typedef std::vector<TWatch> TWatches;
+	typedef std::vector<std::string> TWatchesStr;
+
+	const TWatches& GetWatches() { return m_Watches; }
+
+	TWatchesStr GetStrings() const;
+	void AddFromStrings(const TWatchesStr& bps);
+
+	const bool IsAddressWatch(u32 _iAddress);
+
+	// Add BreakPoint
+	void Add(u32 em_address);
+	void Add(const TWatch& bp);
+
+	void Update(int count, u32 em_address);
+	void UpdateName(int count, const std::string name);
+
+	// Remove Breakpoint
+	void Remove(u32 _iAddress);
+	void Clear();
+
+	void DeleteByAddress(u32 _Address);
+
+private:
+	TWatches m_Watches;
+};

Source/Core/Common/DebugInterface.h

@@ -18,6 +18,7 @@ class DebugInterface
 	virtual void ClearBreakpoint(unsigned int /*address*/){}
 	virtual void ClearAllBreakpoints() {}
 	virtual void ToggleBreakpoint(unsigned int /*address*/){}
+	virtual void AddWatch(unsigned int /*address*/){}
 	virtual void ClearAllMemChecks() {}
 	virtual bool IsMemCheck(unsigned int /*address*/) {return false;}
 	virtual void ToggleMemCheck(unsigned int /*address*/){}

Source/Core/Core/Debugger/PPCDebugInterface.cpp

@@ -131,6 +131,11 @@ void PPCDebugInterface::ToggleBreakpoint(unsigned int address)
 		PowerPC::breakpoints.Add(address);
 }
 
+void PPCDebugInterface::AddWatch(unsigned int address)
+{
+	PowerPC::watches.Add(address);
+}
+
 void PPCDebugInterface::ClearAllMemChecks()
 {
 	PowerPC::memchecks.Clear();

Source/Core/Core/Debugger/PPCDebugInterface.h

@@ -22,6 +22,7 @@ class PPCDebugInterface final : public DebugInterface
 	virtual void SetBreakpoint(unsigned int address) override;
 	virtual void ClearBreakpoint(unsigned int address) override;
 	virtual void ClearAllBreakpoints() override;
+	virtual void AddWatch(unsigned int address) override;
 	virtual void ToggleBreakpoint(unsigned int address) override;
 	virtual void ClearAllMemChecks() override;
 	virtual bool IsMemCheck(unsigned int address) override;

Source/Core/Core/HW/CPU.cpp

@@ -117,6 +117,15 @@ void CCPU::EnableStepping(const bool _bStepping)
 	}
 	else
 	{
+		// SingleStep so that the "continue", "step over" and "step out" debugger functions
+		// work when the PC is at a breakpoint at the beginning of the block
+		if (PowerPC::breakpoints.IsAddressBreakPoint(PC) && PowerPC::GetMode() != PowerPC::MODE_INTERPRETER)
+		{
+			PowerPC::CoreMode oldMode = PowerPC::GetMode();
+			PowerPC::SetMode(PowerPC::MODE_INTERPRETER);
+			PowerPC::SingleStep();
+			PowerPC::SetMode(oldMode);
+		}
 		PowerPC::Start();
 		m_StepEvent.Set();
 		g_video_backend->EmuStateChange(EMUSTATE_CHANGE_PLAY);

Source/Core/Core/PowerPC/Jit64/Jit.cpp

@@ -174,13 +174,8 @@ bool Jit64::HandleFault(uintptr_t access_address, SContext* ctx)
 void Jit64::Init()
 {
 	jo.optimizeStack = true;
-	jo.enableBlocklink = true;
-	if (SConfig::GetInstance().m_LocalCoreStartupParameter.bJITNoBlockLinking ||
-		SConfig::GetInstance().m_LocalCoreStartupParameter.bMMU)
-	{
-		// TODO: support block linking with MMU
-		jo.enableBlocklink = false;
-	}
+	EnableBlockLink();
+
 	jo.fpAccurateFcmp = SConfig::GetInstance().m_LocalCoreStartupParameter.bFPRF;
 	jo.optimizeGatherPipe = true;
 	jo.fastInterrupts = false;
@@ -195,7 +190,7 @@ void Jit64::Init()
 
 	// BLR optimization has the same consequences as block linking, as well as
 	// depending on the fault handler to be safe in the event of excessive BL.
-	m_enable_blr_optimization = jo.enableBlocklink && SConfig::GetInstance().m_LocalCoreStartupParameter.bFastmem;
+	m_enable_blr_optimization = jo.enableBlocklink && SConfig::GetInstance().m_LocalCoreStartupParameter.bFastmem && !SConfig::GetInstance().m_LocalCoreStartupParameter.bEnableDebugging;
 	m_clear_cache_asap = false;
 
 	m_stack = nullptr;
@@ -212,9 +207,7 @@ void Jit64::Init()
 	code_block.m_stats = &js.st;
 	code_block.m_gpa = &js.gpa;
 	code_block.m_fpa = &js.fpa;
-	analyzer.SetOption(PPCAnalyst::PPCAnalyzer::OPTION_CONDITIONAL_CONTINUE);
-	analyzer.SetOption(PPCAnalyst::PPCAnalyzer::OPTION_BRANCH_MERGE);
-	analyzer.SetOption(PPCAnalyst::PPCAnalyzer::OPTION_CARRY_MERGE);
+	EnableOptimization();
 }
 
 void Jit64::ClearCache()
@@ -521,11 +514,23 @@ const u8* Jit64::DoJit(u32 em_address, PPCAnalyst::CodeBuffer *code_buf, JitBloc
 
 	if (SConfig::GetInstance().m_LocalCoreStartupParameter.bEnableDebugging)
 	{
+		// We can link blocks as long as we are not single stepping and there are no breakpoints here
+		EnableBlockLink();
+		EnableOptimization();
+
 		// Comment out the following to disable breakpoints (speed-up)
 		if (!Profiler::g_ProfileBlocks)
 		{
 			if (GetState() == CPU_STEPPING)
+			{
 				blockSize = 1;
+
+				// Do not link this block to other blocks While single stepping
+				jo.enableBlocklink = false;
+				analyzer.ClearOption(PPCAnalyst::PPCAnalyzer::OPTION_CONDITIONAL_CONTINUE);
+				analyzer.ClearOption(PPCAnalyst::PPCAnalyzer::OPTION_BRANCH_MERGE);
+				analyzer.ClearOption(PPCAnalyst::PPCAnalyzer::OPTION_CARRY_MERGE);
+			}
 			Trace();
 		}
 	}
@@ -715,6 +720,9 @@ const u8* Jit64::DoJit(u32 em_address, PPCAnalyst::CodeBuffer *code_buf, JitBloc
 
 			if (SConfig::GetInstance().m_LocalCoreStartupParameter.bEnableDebugging && breakpoints.IsAddressBreakPoint(ops[i].address) && GetState() != CPU_STEPPING)
 			{
+				// Turn off block linking if there are breakpoints so that the Step Over command does not link this block.
+				jo.enableBlocklink = false;
+
 				gpr.Flush();
 				fpr.Flush();
 
@@ -856,3 +864,21 @@ u32 Jit64::CallerSavedRegistersInUse()
 	}
 	return result & ABI_ALL_CALLER_SAVED;
 }
+
+void Jit64::EnableBlockLink()
+{
+	jo.enableBlocklink = true;
+	if (SConfig::GetInstance().m_LocalCoreStartupParameter.bJITNoBlockLinking ||
+	    SConfig::GetInstance().m_LocalCoreStartupParameter.bMMU)
+	{
+		// TODO: support block linking with MMU
+		jo.enableBlocklink = false;
+	}
+}
+
+void Jit64::EnableOptimization()
+{
+	analyzer.SetOption(PPCAnalyst::PPCAnalyzer::OPTION_CONDITIONAL_CONTINUE);
+	analyzer.SetOption(PPCAnalyst::PPCAnalyzer::OPTION_BRANCH_MERGE);
+	analyzer.SetOption(PPCAnalyst::PPCAnalyzer::OPTION_CARRY_MERGE);
+}

Source/Core/Core/PowerPC/Jit64/Jit.h

@@ -64,6 +64,11 @@ class Jit64 : public Jitx86Base
 	~Jit64() {}
 
 	void Init() override;
+
+	void EnableOptimization();
+
+	void EnableBlockLink();
+
 	void Shutdown() override;
 
 	bool HandleFault(uintptr_t access_address, SContext* ctx) override;

Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp

@@ -98,9 +98,12 @@ void Jit64::lXXx(UGeckoInstruction inst)
 	if (accessSize == 8 && js.next_inst.OPCD == 31 && js.next_inst.SUBOP10 == 954 &&
 	    js.next_inst.RS == inst.RD && js.next_inst.RA == inst.RD && !js.next_inst.Rc)
 	{
-		js.downcountAmount++;
-		js.skipnext = true;
-		signExtend = true;
+		if (PowerPC::GetState() != PowerPC::CPU_STEPPING)
+		{
+			js.downcountAmount++;
+			js.skipnext = true;
+			signExtend = true;
+		}
 	}
 
 	// TODO(ector): Make it dynamically enable/disable idle skipping where appropriate
@@ -109,6 +112,7 @@ void Jit64::lXXx(UGeckoInstruction inst)
 	// IMHO those Idles should always be skipped and replaced by a more controllable "native" Idle methode
 	// ... maybe the throttle one already do that :p
 	if (SConfig::GetInstance().m_LocalCoreStartupParameter.bSkipIdle &&
+	    PowerPC::GetState() != PowerPC::CPU_STEPPING &&
 	    inst.OPCD == 32 &&
 	    (inst.hex & 0xFFFF0000) == 0x800D0000 &&
 	    (Memory::ReadUnchecked_U32(js.compilerPC + 4) == 0x28000000 ||

Source/Core/Core/PowerPC/Jit64/Jit_SystemRegisters.cpp

@@ -228,21 +228,24 @@ void Jit64::mfspr(UGeckoInstruction inst)
 		// Be careful; the actual opcode is for mftb (371), not mfspr (339)
 		if (js.next_inst.OPCD == 31 && js.next_inst.SUBOP10 == 371 && (nextIndex == SPR_TU || nextIndex == SPR_TL))
 		{
-			int n = js.next_inst.RD;
-			js.downcountAmount++;
-			js.skipnext = true;
-			gpr.Lock(d, n);
-			gpr.BindToRegister(d, false);
-			gpr.BindToRegister(n, false);
-			if (iIndex == SPR_TL)
-				MOV(32, gpr.R(d), R(RAX));
-			if (nextIndex == SPR_TL)
-				MOV(32, gpr.R(n), R(RAX));
-			SHR(64, R(RAX), Imm8(32));
-			if (iIndex == SPR_TU)
-				MOV(32, gpr.R(d), R(RAX));
-			if (nextIndex == SPR_TU)
-				MOV(32, gpr.R(n), R(RAX));
+			if (PowerPC::GetState() != PowerPC::CPU_STEPPING)
+			{
+				int n = js.next_inst.RD;
+				js.downcountAmount++;
+				js.skipnext = true;
+				gpr.Lock(d, n);
+				gpr.BindToRegister(d, false);
+				gpr.BindToRegister(n, false);
+				if (iIndex == SPR_TL)
+					MOV(32, gpr.R(d), R(RAX));
+				if (nextIndex == SPR_TL)
+					MOV(32, gpr.R(n), R(RAX));
+				SHR(64, R(RAX), Imm8(32));
+				if (iIndex == SPR_TU)
+					MOV(32, gpr.R(d), R(RAX));
+				if (nextIndex == SPR_TU)
+					MOV(32, gpr.R(n), R(RAX));
+			}
 		}
 		else
 		{