Merge pull request #8844 from unknownbrackets/ir-debug

Add memory breakpoints to IR as well

Core/Debugger/Breakpoints.cpp

@@ -364,6 +364,31 @@ void CBreakPoints::ExecMemCheck(u32 address, bool write, int size, u32 pc)
 		check->Action(address, write, size, pc);
 }
 
+void CBreakPoints::ExecOpMemCheck(u32 address, u32 pc)
+{
+	// Note: currently, we don't check "on changed" for HLE (ExecMemCheck.)
+	// We'd need to more carefully specify memory changes in HLE for that.
+	int size = MIPSAnalyst::OpMemoryAccessSize(pc);
+	if (size == 0 && MIPSAnalyst::OpHasDelaySlot(pc)) {
+		// This means that the delay slot is what tripped us.
+		pc += 4;
+		size = MIPSAnalyst::OpMemoryAccessSize(pc);
+	}
+
+	bool write = MIPSAnalyst::IsOpMemoryWrite(pc);
+	auto check = GetMemCheck(address, size);
+	if (check) {
+		int mask = MEMCHECK_WRITE | MEMCHECK_WRITE_ONCHANGE;
+		if (write && (check->cond & mask) == mask) {
+			if (MIPSAnalyst::OpWouldChangeMemory(pc, address, size)) {
+				check->Action(address, write, size, pc);
+			}
+		} else {
+			check->Action(address, write, size, pc);
+		}
+	}
+}
+
 void CBreakPoints::ExecMemCheckJitBefore(u32 address, bool write, int size, u32 pc)
 {
 	auto check = GetMemCheck(address, size);
@@ -421,6 +446,11 @@ const std::vector<BreakPoint> CBreakPoints::GetBreakpoints()
 	return breakPoints_;
 }
 
+bool CBreakPoints::HasMemChecks()
+{
+	return !memChecks_.empty();
+}
+
 void CBreakPoints::Update(u32 addr)
 {
 	if (MIPSComp::jit)

Core/Debugger/Breakpoints.h

@@ -134,6 +134,7 @@ class CBreakPoints
 
 	static MemCheck *GetMemCheck(u32 address, int size);
 	static void ExecMemCheck(u32 address, bool write, int size, u32 pc);
+	static void ExecOpMemCheck(u32 address, u32 pc);
 
 	// Executes memchecks but used by the jit.  Cleanup finalizes after jit is done.
 	static void ExecMemCheckJitBefore(u32 address, bool write, int size, u32 pc);
@@ -148,6 +149,8 @@ class CBreakPoints
 	static const std::vector<MemCheck> GetMemChecks();
 	static const std::vector<BreakPoint> GetBreakpoints();
 
+	static bool HasMemChecks();
+
 	static void Update(u32 addr = 0);
 
 private:

Core/MIPS/IR/IRCompBranch.cpp

@@ -75,10 +75,6 @@ void IRFrontend::BranchRSRTComp(MIPSOpcode op, IRComparison cc, bool likely) {
 		return;
 	}
 
-	int dcAmount = js.downcountAmount;
-	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
-	js.downcountAmount = 0;
-
 	MIPSGPReg lhs = rs;
 	MIPSGPReg rhs = rt;
 	if (!delaySlotIsNice && !likely) {  // if likely, we don't need this
@@ -95,6 +91,10 @@ void IRFrontend::BranchRSRTComp(MIPSOpcode op, IRComparison cc, bool likely) {
 	if (!likely)
 		CompileDelaySlot();
 
+	int dcAmount = js.downcountAmount;
+	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
+	js.downcountAmount = 0;
+
 	FlushAll();
 	ir.Write(ComparisonToExit(cc), ir.AddConstant(GetCompilerPC() + 8), lhs, rhs);
 	// This makes the block "impure" :(
@@ -119,10 +119,6 @@ void IRFrontend::BranchRSZeroComp(MIPSOpcode op, IRComparison cc, bool andLink,
 	MIPSOpcode delaySlotOp = GetOffsetInstruction(1);
 	bool delaySlotIsNice = IsDelaySlotNiceReg(op, delaySlotOp, rs);
 
-	int dcAmount = js.downcountAmount;
-	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
-	js.downcountAmount = 0;
-
 	MIPSGPReg lhs = rs;
 	if (!delaySlotIsNice) {  // if likely, we don't need this
 		ir.Write(IROp::Mov, IRTEMP_LHS, rs);
@@ -134,6 +130,10 @@ void IRFrontend::BranchRSZeroComp(MIPSOpcode op, IRComparison cc, bool andLink,
 	if (!likely)
 		CompileDelaySlot();
 
+	int dcAmount = js.downcountAmount;
+	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
+	js.downcountAmount = 0;
+
 	FlushAll();
 	ir.Write(ComparisonToExit(cc), ir.AddConstant(GetCompilerPC() + 8), lhs);
 	if (likely)
@@ -233,22 +233,22 @@ void IRFrontend::BranchVFPUFlag(MIPSOpcode op, IRComparison cc, bool likely) {
 	MIPSOpcode delaySlotOp = GetOffsetInstruction(1);
 	ir.Write(IROp::VfpuCtrlToReg, IRTEMP_LHS, VFPU_CTRL_CC);
 
-	int dcAmount = js.downcountAmount;
-	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
-	js.downcountAmount = 0;
-
 	// Sometimes there's a VFPU branch in a delay slot (Disgaea 2: Dark Hero Days, Zettai Hero Project, La Pucelle)
 	// The behavior is undefined - the CPU may take the second branch even if the first one passes.
 	// However, it does consistently try each branch, which these games seem to expect.
 	bool delaySlotIsBranch = MIPSCodeUtils::IsVFPUBranch(delaySlotOp);
 	if (!likely)
 		CompileDelaySlot();
 
+	int dcAmount = js.downcountAmount;
+	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
+	js.downcountAmount = 0;
+
 	if (delaySlotIsBranch && (signed short)(delaySlotOp & 0xFFFF) != (signed short)(op & 0xFFFF) - 1)
 		ERROR_LOG_REPORT(JIT, "VFPU branch in VFPU delay slot at %08x with different target", GetCompilerPC());
 
 	int imm3 = (op >> 18) & 7;
-	
+
 	u32 notTakenTarget = GetCompilerPC() + (delaySlotIsBranch ? 4 : 8);
 
 	ir.Write(IROp::AndConst, IRTEMP_LHS, IRTEMP_LHS, ir.AddConstant(1 << imm3));
@@ -282,10 +282,6 @@ void IRFrontend::Comp_Jump(MIPSOpcode op) {
 	u32 off = _IMM26 << 2;
 	u32 targetAddr = (GetCompilerPC() & 0xF0000000) | off;
 
-	int dcAmount = js.downcountAmount;
-	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
-	js.downcountAmount = 0;
-
 	// Might be a stubbed address or something?
 	if (!Memory::IsValidAddress(targetAddr)) {
 		if (js.nextExit == 0) {
@@ -300,21 +296,24 @@ void IRFrontend::Comp_Jump(MIPSOpcode op) {
 	switch (op >> 26) {
 	case 2: //j
 		CompileDelaySlot();
-		FlushAll();
-		ir.Write(IROp::ExitToConst, ir.AddConstant(targetAddr));
 		break;
 
 	case 3: //jal
 		ir.WriteSetConstant(MIPS_REG_RA, GetCompilerPC() + 8);
 		CompileDelaySlot();
-		FlushAll();
-		ir.Write(IROp::ExitToConst, ir.AddConstant(targetAddr));
 		break;
 
 	default:
 		_dbg_assert_msg_(CPU,0,"Trying to compile instruction that can't be compiled");
 		break;
 	}
+
+	int dcAmount = js.downcountAmount;
+	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
+	js.downcountAmount = 0;
+
+	FlushAll();
+	ir.Write(IROp::ExitToConst, ir.AddConstant(targetAddr));
 	js.compiling = false;
 }
 
@@ -332,10 +331,6 @@ void IRFrontend::Comp_JumpReg(MIPSOpcode op) {
 	if (andLink && rs == rd)
 		delaySlotIsNice = false;
 
-	int dcAmount = js.downcountAmount;
-	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
-	js.downcountAmount = 0;
-
 	int destReg;
 	if (IsSyscall(delaySlotOp)) {
 		ir.Write(IROp::SetPC, 0, rs);
@@ -360,7 +355,7 @@ void IRFrontend::Comp_JumpReg(MIPSOpcode op) {
 		FlushAll();
 	}
 
-	switch (op & 0x3f) 
+	switch (op & 0x3f)
 	{
 	case 8: //jr
 		break;
@@ -371,13 +366,17 @@ void IRFrontend::Comp_JumpReg(MIPSOpcode op) {
 		break;
 	}
 
+	int dcAmount = js.downcountAmount;
+	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
+	js.downcountAmount = 0;
+
 	ir.Write(IROp::ExitToReg, 0, destReg, 0);
 	js.compiling = false;
 }
 
 void IRFrontend::Comp_Syscall(MIPSOpcode op) {
 	// Note: If we're in a delay slot, this is off by one compared to the interpreter.
-	int dcAmount = js.downcountAmount;
+	int dcAmount = js.downcountAmount + (js.inDelaySlot ? -1 : 0);
 	ir.Write(IROp::Downcount, 0, dcAmount & 0xFF, dcAmount >> 8);
 	js.downcountAmount = 0;
 

Core/MIPS/IR/IRCompFPU.cpp

@@ -78,6 +78,8 @@ void IRFrontend::Comp_FPULS(MIPSOpcode op) {
 	int ft = _FT;
 	MIPSGPReg rs = _RS;
 
+	CheckMemoryBreakpoint(rs, offset);
+
 	switch (op >> 26) {
 	case 49: //FI(ft) = Memory::Read_U32(addr); break; //lwc1
 		ir.Write(IROp::LoadFloat, ft, rs, ir.AddConstant(offset));

Core/MIPS/IR/IRCompLoadStore.cpp

@@ -15,30 +15,8 @@
 // Official git repository and contact information can be found at
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
-
-// Optimization ideas:
-//
-// It's common to see sequences of stores writing or reading to a contiguous set of
-// addresses in function prologues/epilogues:
-//  sw s5, 104(sp)
-//  sw s4, 100(sp)
-//  sw s3, 96(sp)
-//  sw s2, 92(sp)
-//  sw s1, 88(sp)
-//  sw s0, 84(sp)
-//  sw ra, 108(sp)
-//  mov s4, a0
-//  mov s3, a1
-//  ...
-// Such sequences could easily be detected and turned into nice contiguous
-// sequences of ARM stores instead of the current 3 instructions per sw/lw.
-//
-// Also, if we kept track of the likely register content of a cached register,
-// (pointer or data), we could avoid many BIC instructions.
-
-
-#include "Core/MemMap.h"
 #include "Core/Config.h"
+#include "Core/MemMap.h"
 #include "Core/MIPS/MIPS.h"
 #include "Core/MIPS/MIPSAnalyst.h"
 #include "Core/MIPS/MIPSCodeUtils.h"
@@ -81,6 +59,8 @@ namespace MIPSComp {
 			return;
 		}
 
+		CheckMemoryBreakpoint(rs, offset);
+
 		int addrReg = IRTEMP_0;
 		switch (o) {
 			// Load

Core/MIPS/IR/IRCompVFPU.cpp

@@ -19,17 +19,17 @@
 
 #include "math/math_util.h"
 
+#include "Common/CPUDetect.h"
+#include "Core/Config.h"
 #include "Core/MemMap.h"
 #include "Core/MIPS/MIPS.h"
 #include "Core/MIPS/MIPSTables.h"
 #include "Core/MIPS/MIPSAnalyst.h"
 #include "Core/MIPS/MIPSCodeUtils.h"
-#include "Common/CPUDetect.h"
-#include "Core/Config.h"
-#include "Core/Reporting.h"
-
 #include "Core/MIPS/IR/IRFrontend.h"
 #include "Core/MIPS/IR/IRRegCache.h"
+#include "Core/Reporting.h"
+
 
 // All functions should have CONDITIONAL_DISABLE, so we can narrow things down to a file quickly.
 // Currently known non working ones should have DISABLE.
@@ -277,6 +277,9 @@ namespace MIPSComp {
 		s32 offset = (signed short)(op & 0xFFFC);
 		int vt = ((op >> 16) & 0x1f) | ((op & 3) << 5);
 		MIPSGPReg rs = _RS;
+
+		CheckMemoryBreakpoint(rs, offset);
+
 		switch (op >> 26) {
 		case 50: //lv.s
 			ir.Write(IROp::LoadFloat, vfpuBase + voffset[vt], rs, ir.AddConstant(offset));
@@ -300,6 +303,8 @@ namespace MIPSComp {
 		u8 vregs[4];
 		GetVectorRegs(vregs, V_Quad, vt);
 
+		CheckMemoryBreakpoint(rs, imm);
+
 		switch (op >> 26) {
 		case 54: //lv.q
 			if (IsConsecutive4(vregs)) {

Core/MIPS/IR/IRFrontend.cpp

@@ -91,15 +91,15 @@ void IRFrontend::EatInstruction(MIPSOpcode op) {
 		ERROR_LOG_REPORT_ONCE(ateInDelaySlot, JIT, "Ate an instruction inside a delay slot.");
 	}
 
-	CheckBreakpoint(GetCompilerPC() + 4, 0);
+	CheckBreakpoint(GetCompilerPC() + 4);
 	js.numInstructions++;
 	js.compilerPC += 4;
 	js.downcountAmount += MIPSGetInstructionCycleEstimate(op);
 }
 
 void IRFrontend::CompileDelaySlot() {
 	js.inDelaySlot = true;
-	CheckBreakpoint(GetCompilerPC() + 4, -2);
+	CheckBreakpoint(GetCompilerPC() + 4);
 	MIPSOpcode op = GetOffsetInstruction(1);
 	MIPSCompileOp(op, this);
 	js.inDelaySlot = false;
@@ -239,7 +239,7 @@ void IRFrontend::DoJit(u32 em_address, std::vector<IRInst> &instructions, std::v
 	js.numInstructions = 0;
 	while (js.compiling) {
 		// Jit breakpoints are quite fast, so let's do them in release too.
-		CheckBreakpoint(GetCompilerPC(), 0);
+		CheckBreakpoint(GetCompilerPC());
 
 		MIPSOpcode inst = Memory::Read_Opcode_JIT(GetCompilerPC());
 		js.downcountAmount += MIPSGetInstructionCycleEstimate(inst);
@@ -318,21 +318,47 @@ void IRFrontend::Comp_RunBlock(MIPSOpcode op) {
 	ERROR_LOG(JIT, "Comp_RunBlock should never be reached!");
 }
 
-void IRFrontend::CheckBreakpoint(u32 addr, int downcountOffset) {
+void IRFrontend::CheckBreakpoint(u32 addr) {
 	if (CBreakPoints::IsAddressBreakPoint(addr)) {
 		FlushAll();
 
 		RestoreRoundingMode();
 		ir.Write(IROp::SetPCConst, 0, ir.AddConstant(GetCompilerPC()));
+		// 0 because we normally execute before increasing.
+		// TODO: In likely branches, downcount will be incorrect.
+		int downcountOffset = js.inDelaySlot && js.downcountAmount >= 2 ? -2 : 0;
 		int downcountAmount = js.downcountAmount + downcountOffset;
 		ir.Write(IROp::Downcount, 0, downcountAmount & 0xFF, downcountAmount >> 8);
 		// Note that this means downcount can't be metadata on the block.
-		js.downcountAmount = -downcountAmount;
+		js.downcountAmount = -downcountOffset;
 		ir.Write(IROp::Breakpoint);
 		ApplyRoundingMode();
 
 		js.hadBreakpoints = true;
 	}
 }
 
+void IRFrontend::CheckMemoryBreakpoint(int rs, int offset) {
+	if (CBreakPoints::HasMemChecks()) {
+		FlushAll();
+
+		RestoreRoundingMode();
+		ir.Write(IROp::SetPCConst, 0, ir.AddConstant(GetCompilerPC()));
+		// 0 because we normally execute before increasing.
+		int downcountOffset = js.inDelaySlot ? -2 : -1;
+		// TODO: In likely branches, downcount will be incorrect.  This might make resume fail.
+		if (js.downcountAmount == 0) {
+			downcountOffset = 0;
+		}
+		int downcountAmount = js.downcountAmount + downcountOffset;
+		ir.Write(IROp::Downcount, 0, downcountAmount & 0xFF, downcountAmount >> 8);
+		// Note that this means downcount can't be metadata on the block.
+		js.downcountAmount = -downcountOffset;
+		ir.Write(IROp::MemoryCheck, 0, rs, ir.AddConstant(offset));
+		ApplyRoundingMode();
+
+		js.hadBreakpoints = true;
+	}
+}
+
 }  // namespace

Core/MIPS/IR/IRFrontend.h

@@ -107,7 +107,8 @@ class IRFrontend : public MIPSFrontendInterface {
 	void EatInstruction(MIPSOpcode op);
 	MIPSOpcode GetOffsetInstruction(int offset);
 
-	void CheckBreakpoint(u32 addr, int downcountOffset);
+	void CheckBreakpoint(u32 addr);
+	void CheckMemoryBreakpoint(int rs, int offset);
 
 	// Utility compilation functions
 	void BranchFPFlag(MIPSOpcode op, IRComparison cc, bool likely);

Core/MIPS/IR/IRInst.cpp

@@ -156,7 +156,8 @@ static const IRMeta irMeta[] = {
 	{ IROp::SetPC, "SetPC", "_G" },
 	{ IROp::SetPCConst, "SetPC", "_C" },
 	{ IROp::CallReplacement, "CallRepl", "_C" },
-	{ IROp::Breakpoint, "Breakpoint", "" },
+	{ IROp::Breakpoint, "Breakpoint", "", IRFLAG_EXIT },
+	{ IROp::MemoryCheck, "MemoryCheck", "_GC", IRFLAG_EXIT },
 };
 
 const IRMeta *metaIndex[256];