Merge pull request #19449 from hrydgard/ir-interpreter-breakpoints

IR interpreter: Replace the assert for downcount with a very predictable branch

Core/MIPS/IR/IRJit.cpp

@@ -104,7 +104,7 @@ void IRJit::InvalidateCacheAt(u32 em_address, int length) {
 		// INFO_LOG(Log::JIT, "Block at %08x invalidated: valid: %d", block->GetOriginalStart(), block->IsValid());
 		// If we're a native JIT (IR->JIT, not just IR interpreter), we write native offsets into the blocks.
 		int cookie = compileToNative_ ? block->GetNativeOffset() : block->GetIRArenaOffset();
-		blocks_.RemoveBlock(block_num);
+		blocks_.RemoveBlockFromPageLookup(block_num);
 		block->Destroy(cookie);
 	}
 }
@@ -279,10 +279,12 @@ void IRJit::RunLoopUntil(u64 globalticks) {
 			if (opcode == MIPS_EMUHACK_OPCODE) {
 				u32 offset = inst & 0x00FFFFFF; // Alternatively, inst - opcode
 				const IRInst *instPtr = blocks_.GetArenaPtr() + offset;
-				// First op is always downcount, to save one dispatch.
-				_dbg_assert_(instPtr->op == IROp::Downcount);
-				mips->downcount -= instPtr->constant;
-				instPtr++;
+				// First op is always, except when using breakpoints, downcount, to save one dispatch inside IRInterpret.
+				// This branch is very cpu-branch-predictor-friendly so this still beats the dispatch.
+				if (instPtr->op == IROp::Downcount) {
+					mips->downcount -= instPtr->constant;
+					instPtr++;
+				}
 #ifdef IR_PROFILING
 				IRBlock *block = blocks_.GetBlock(blocks_.GetBlockNumFromOffset(offset));
 				Instant start = Instant::Now();
@@ -429,7 +431,7 @@ void IRBlockCache::FinalizeBlock(int blockIndex, bool preload) {
 	}
 
 	u32 startAddr, size;
-	block.GetRange(startAddr, size);
+	block.GetRange(&startAddr, &size);
 
 	u32 startPage = AddressToPage(startAddr);
 	u32 endPage = AddressToPage(startAddr + size);
@@ -440,12 +442,12 @@ void IRBlockCache::FinalizeBlock(int blockIndex, bool preload) {
 }
 
 // Call after Destroy-ing it.
-void IRBlockCache::RemoveBlock(int blockIndex) {
+void IRBlockCache::RemoveBlockFromPageLookup(int blockIndex) {
 	// We need to remove the block from the byPage lookup.
 	IRBlock &block = blocks_[blockIndex];
 
 	u32 startAddr, size;
-	block.GetRange(startAddr, size);
+	block.GetRange(&startAddr, &size);
 
 	u32 startPage = AddressToPage(startAddr);
 	u32 endPage = AddressToPage(startAddr + size);
@@ -454,7 +456,8 @@ void IRBlockCache::RemoveBlock(int blockIndex) {
 		auto iter = std::find(byPage_[page].begin(), byPage_[page].end(), blockIndex);
 		if (iter != byPage_[page].end()) {
 			byPage_[page].erase(iter);
-		} else {
+		} else if (block.IsValid()) {
+			// If it was previously invalidated, we don't care, hence the above check.
 			WARN_LOG(Log::JIT, "RemoveBlock: Block at %08x was not found where expected in byPage table.", startAddr);
 		}
 	}
@@ -547,7 +550,7 @@ JitBlockDebugInfo IRBlockCache::GetBlockDebugInfo(int blockNum) const {
 	const IRBlock &ir = blocks_[blockNum];
 	JitBlockDebugInfo debugInfo{};
 	uint32_t start, size;
-	ir.GetRange(start, size);
+	ir.GetRange(&start, &size);
 	debugInfo.originalAddress = start;  // TODO
 
 	debugInfo.origDisasm.reserve(((start + size) - start) / 4);
@@ -578,7 +581,7 @@ void IRBlockCache::ComputeStats(BlockCacheStats &bcStats) const {
 		if (codeSize == 0)
 			continue;
 		u32 origAddr, mipsBytes;
-		b.GetRange(origAddr, mipsBytes);
+		b.GetRange(&origAddr, &mipsBytes);
 		double origSize = (double)mipsBytes;
 		double bloat = codeSize / origSize;
 		if (bloat < minBloat) {
@@ -650,7 +653,7 @@ void IRBlock::Destroy(int cookie) {
 			Memory::Write_Opcode_JIT(origAddr_, origFirstOpcode_);
 		} else {
 			// NOTE: This is not an error. Just interesting to log.
-			DEBUG_LOG(Log::JIT, "IRBlock::Destroy: Block at %08x was overwritten - expected %08x, got %08x when restoring the MIPS op to %08x", origAddr_, opcode.encoding, memOp, origFirstOpcode_.encoding);
+			DEBUG_LOG(Log::JIT, "IRBlock::Destroy: Note: Block at %08x was overwritten - checked for %08x, got %08x when restoring the MIPS op to %08x", origAddr_, opcode.encoding, memOp, origFirstOpcode_.encoding);
 		}
 		// TODO: Also wipe the block in the IR opcode arena.
 		// Let's mark this invalid so we don't try to clear it again.

Core/MIPS/IR/IRJit.h

@@ -84,9 +84,9 @@ class IRBlock {
 	}
 	bool OverlapsRange(u32 addr, u32 size) const;
 
-	void GetRange(u32 &start, u32 &size) const {
-		start = origAddr_;
-		size = origSize_;
+	void GetRange(u32 *start, u32 *size) const {
+		*start = origAddr_;
+		*size = origSize_;
 	}
 	u32 GetOriginalStart() const {
 		return origAddr_;
@@ -129,7 +129,7 @@ class IRBlockCache : public JitBlockCacheDebugInterface {
 			return nullptr;
 		}
 	}
-	void RemoveBlock(int blockNum);
+	void RemoveBlockFromPageLookup(int blockNum);
 	int GetBlockNumFromIRArenaOffset(int offset) const;
 	const IRInst *GetBlockInstructionPtr(const IRBlock &block) const {
 		return arena_.data() + block.GetIRArenaOffset();
@@ -167,7 +167,7 @@ class IRBlockCache : public JitBlockCacheDebugInterface {
 		JitBlockMeta meta{};
 		if (IsValidBlock(blockNum)) {
 			meta.valid = true;
-			blocks_[blockNum].GetRange(meta.addr, meta.sizeInBytes);
+			blocks_[blockNum].GetRange(&meta.addr, &meta.sizeInBytes);
 		}
 		return meta;
 	}

Core/MIPS/IR/IRNativeCommon.cpp

@@ -570,7 +570,7 @@ bool IRNativeJit::DescribeCodePtr(const u8 *ptr, std::string &name) {
 	const IRBlock *block = blocks_.GetBlock(block_num);
 	if (block) {
 		u32 start = 0, size = 0;
-		block->GetRange(start, size);
+		block->GetRange(&start, &size);
 
 		// It helps to know which func this block is inside.
 		const std::string label = g_symbolMap ? g_symbolMap->GetDescription(start) : "";
@@ -782,7 +782,7 @@ void IRNativeBlockCacheDebugInterface::ComputeStats(BlockCacheStats &bcStats) co
 
 		// MIPS (PSP) size.
 		u32 origAddr, origSize;
-		b.GetRange(origAddr, origSize);
+		b.GetRange(&origAddr, &origSize);
 
 		double bloat = (double)codeSize / (double)origSize;
 		if (bloat < minBloat) {