Merge pull request #23025 from hrydgard/branch-in-delay-slot-test

CPU: Branches and jumps in delay slots as on hardware

Core/MIPS/IR/IRCompBranch.cpp

@@ -54,11 +54,48 @@ using namespace MIPSAnalyst;
 namespace MIPSComp
 {
 
-void IRFrontend::BranchRSRTComp(MIPSOpcode op, IRComparison cc, bool likely) {
-	if (js.inDelaySlot) {
-		ERROR_LOG_REPORT(Log::JIT, "Branch in RSRTComp delay slot at %08x in block starting at %08x", GetCompilerPC(), js.blockStart);
-		return;
+// A branch or jump with another in its delay slot is left to the interpreter, which does what the
+// hardware does (tests/cpu/delayslot/branch). It's rare, and mostly data run as code. Ends the block.
+bool IRFrontend::CompileBranchPair(MIPSOpcode op) {
+	const MIPSOpcode delaySlotOp = GetOffsetInstruction(1);
+	const MIPSInfo delaySlotInfo = MIPSGetInfo(delaySlotOp);
+	if ((delaySlotInfo & (IS_JUMP | IS_CONDBRANCH)) == 0) {
+		return false;
 	}
+	// A jump skipped by a likely branch runs the next instruction as its delay slot, which may be
+	// anything. Handled as Comp_Generic does.
+	MIPSInfo afterInfo;
+	u32 afterOp = 0;
+	if ((MIPSGetInfo(op) & LIKELY) && (delaySlotInfo & IS_JUMP)) {
+		afterOp = GetOffsetInstruction(2).encoding;
+		afterInfo = MIPSGetInfo(MIPSOpcode(afterOp));
+	}
+	const bool isSyscall = (afterInfo & IS_SYSCALL) != 0;
+	const bool setsMode = MIPS_IS_CTC1_FCR31(afterOp);
+
+	// The interpreter charges the branch as it runs it.
+	ir.Write(IROp::Downcount, 0, 0, 0, js.downcountAmount - MIPSGetInstructionCycleEstimate(op));
+	js.downcountAmount = 0;
+	FlushAll();
+	if (isSyscall || setsMode)
+		RestoreRoundingMode(setsMode);
+	ir.Write(IROp::SetPCConst, 0, 0, 0, GetCompilerPC());
+	ir.Write(IROp::Interpret, 0, 0, 0, MIPS_BRANCH_PAIR_OP);
+	if (setsMode)
+		UpdateRoundingMode();
+	if (isSyscall || setsMode)
+		ApplyRoundingMode(setsMode);
+	if (afterInfo & OUT_VFPU_PREFIX)
+		js.PrefixUnknown();
+	ir.Write(IROp::ExitToPC);
+	js.compiling = false;
+	return true;
+}
+
+void IRFrontend::BranchRSRTComp(MIPSOpcode op, IRComparison cc, bool likely) {
+	_dbg_assert_(!js.inDelaySlot);
+	if (CompileBranchPair(op))
+		return;
 	int offset = TARGET16;
 	MIPSGPReg rt = _RT;
 	MIPSGPReg rs = _RS;
@@ -92,7 +129,7 @@ void IRFrontend::BranchRSRTComp(MIPSOpcode op, IRComparison cc, bool likely) {
 		}
 	}
 
-	if (!likely && !branchInfo.delaySlotIsBranch)
+	if (!likely)
 		CompileDelaySlot();
 
 	int dcAmount = js.downcountAmount;
@@ -105,18 +142,10 @@ void IRFrontend::BranchRSRTComp(MIPSOpcode op, IRComparison cc, bool likely) {
 		js.compilerPC += 4;
 		return;
 	}
-	ir.Write(ComparisonToExit(cc), 0, lhs, rhs, ResolveNotTakenTarget(branchInfo));
+	ir.Write(ComparisonToExit(cc), 0, lhs, rhs, GetCompilerPC() + 8);
 	// This makes the block "impure" :(
-	if (likely && !branchInfo.delaySlotIsBranch)
+	if (likely)
 		CompileDelaySlot();
-	if (branchInfo.delaySlotIsBranch) {
-		// We still link when the branch is taken (targetAddr case.)
-		// Remember, it's from the perspective of the delay slot, so +12.
-		if ((branchInfo.delaySlotInfo & OUT_RA) != 0)
-			ir.WriteSetConstant(MIPS_REG_RA, GetCompilerPC() + 12);
-		if ((branchInfo.delaySlotInfo & OUT_RD) != 0)
-			ir.WriteSetConstant(MIPS_GET_RD(branchInfo.delaySlotOp), GetCompilerPC() + 12);
-	}
 
 	FlushAll();
 	ir.Write(IROp::ExitToConst, 0, 0, 0, targetAddr);
@@ -127,10 +156,9 @@ void IRFrontend::BranchRSRTComp(MIPSOpcode op, IRComparison cc, bool likely) {
 }
 
 void IRFrontend::BranchRSZeroComp(MIPSOpcode op, IRComparison cc, bool andLink, bool likely) {
-	if (js.inDelaySlot) {
-		ERROR_LOG_REPORT(Log::JIT, "Branch in RSZeroComp delay slot at %08x in block starting at %08x", GetCompilerPC(), js.blockStart);
+	_dbg_assert_(!js.inDelaySlot);
+	if (CompileBranchPair(op))
 		return;
-	}
 	int offset = TARGET16;
 	MIPSGPReg rs = _RS;
 	u32 targetAddr = GetCompilerPC() + offset + 4;
@@ -149,7 +177,7 @@ void IRFrontend::BranchRSZeroComp(MIPSOpcode op, IRComparison cc, bool andLink,
 	if (andLink)
 		ir.WriteSetConstant(MIPS_REG_RA, GetCompilerPC() + 8);
 
-	if (!likely && !branchInfo.delaySlotIsBranch)
+	if (!likely)
 		CompileDelaySlot();
 
 	int dcAmount = js.downcountAmount;
@@ -162,17 +190,9 @@ void IRFrontend::BranchRSZeroComp(MIPSOpcode op, IRComparison cc, bool andLink,
 		js.compilerPC += 4;
 		return;
 	}
-	ir.Write(ComparisonToExit(cc), 0, lhs, 0, ResolveNotTakenTarget(branchInfo));
-	if (likely && !branchInfo.delaySlotIsBranch)
+	ir.Write(ComparisonToExit(cc), 0, lhs, 0, GetCompilerPC() + 8);
+	if (likely)
 		CompileDelaySlot();
-	if (branchInfo.delaySlotIsBranch) {
-		// We still link when the branch is taken (targetAddr case.)
-		// Remember, it's from the perspective of the delay slot, so +12.
-		if ((branchInfo.delaySlotInfo & OUT_RA) != 0)
-			ir.WriteSetConstant(MIPS_REG_RA, GetCompilerPC() + 12);
-		if ((branchInfo.delaySlotInfo & OUT_RD) != 0)
-			ir.WriteSetConstant(MIPS_GET_RD(branchInfo.delaySlotOp), GetCompilerPC() + 12);
-	}
 
 	// Taken
 	FlushAll();
@@ -190,7 +210,7 @@ void IRFrontend::BranchRSZeroComp(MIPSOpcode op, IRComparison cc, bool andLink,
 // usually followed by something else entirely). Nor with a prefix pending: blocks assume default
 // prefixes on entry, and only the end of a block checks that it leaves them so (CheckRounding()).
 bool IRFrontend::CanContinuePastBranch(const BranchInfo &branchInfo, bool constantOperands) {
-	if (!opts.continueBranches || branchInfo.likely || branchInfo.delaySlotIsBranch || constantOperands || js.MayHavePrefix())
+	if (!opts.continueBranches || branchInfo.likely || constantOperands || js.MayHavePrefix())
 		return false;
 	if (js.numInstructions >= IR_MAX_CONTINUED_INSTRUCTIONS || js.continuedBranches >= IR_MAX_CONTINUED_BRANCHES)
 		return false;
@@ -237,18 +257,17 @@ void IRFrontend::Comp_RelBranchRI(MIPSOpcode op) {
 
 // If likely is set, discard the branch slot if NOT taken.
 void IRFrontend::BranchFPFlag(MIPSOpcode op, IRComparison cc, bool likely) {
-	if (js.inDelaySlot) {
-		ERROR_LOG_REPORT(Log::JIT, "Branch in FPFlag delay slot at %08x in block starting at %08x", GetCompilerPC(), js.blockStart);
+	_dbg_assert_(!js.inDelaySlot);
+	if (CompileBranchPair(op))
 		return;
-	}
 	int offset = TARGET16;
 	u32 targetAddr = GetCompilerPC() + offset + 4;
 
-	BranchInfo branchInfo(GetCompilerPC(), op, GetOffsetInstruction(1), false, likely);
+	MIPSOpcode delaySlotOp = GetOffsetInstruction(1);
 
-	js.downcountAmount += MIPSGetInstructionCycleEstimate(branchInfo.delaySlotOp);
+	js.downcountAmount += MIPSGetInstructionCycleEstimate(delaySlotOp);
 	ir.Write(IROp::FpCondToReg, IRTEMP_LHS);
-	if (!likely && !branchInfo.delaySlotIsBranch)
+	if (!likely)
 		CompileDelaySlot();
 
 	int dcAmount = js.downcountAmount;
@@ -257,18 +276,10 @@ void IRFrontend::BranchFPFlag(MIPSOpcode op, IRComparison cc, bool likely) {
 
 	FlushAll();
 	// Not taken
-	ir.Write(ComparisonToExit(cc), 0, IRTEMP_LHS, 0, ResolveNotTakenTarget(branchInfo));
+	ir.Write(ComparisonToExit(cc), 0, IRTEMP_LHS, 0, GetCompilerPC() + 8);
 	// Taken
-	if (likely && !branchInfo.delaySlotIsBranch)
+	if (likely)
 		CompileDelaySlot();
-	if (branchInfo.delaySlotIsBranch) {
-		// We still link when the branch is taken (targetAddr case.)
-		// Remember, it's from the perspective of the delay slot, so +12.
-		if ((branchInfo.delaySlotInfo & OUT_RA) != 0)
-			ir.WriteSetConstant(MIPS_REG_RA, GetCompilerPC() + 12);
-		if ((branchInfo.delaySlotInfo & OUT_RD) != 0)
-			ir.WriteSetConstant(MIPS_GET_RD(branchInfo.delaySlotOp), GetCompilerPC() + 12);
-	}
 
 	FlushAll();
 	ir.Write(IROp::ExitToConst, 0, 0, 0, targetAddr);
@@ -292,22 +303,19 @@ void IRFrontend::Comp_FPUBranch(MIPSOpcode op) {
 
 // If likely is set, discard the branch slot if NOT taken.
 void IRFrontend::BranchVFPUFlag(MIPSOpcode op, IRComparison cc, bool likely) {
-	if (js.inDelaySlot) {
-		ERROR_LOG_REPORT(Log::JIT, "Branch in VFPU delay slot at %08x in block starting at %08x", GetCompilerPC(), js.blockStart);
+	_dbg_assert_(!js.inDelaySlot);
+	// Disgaea 2: Dark Hero Days, Zettai Hero Project and La Pucelle have VFPU branches in delay slots.
+	if (CompileBranchPair(op))
 		return;
-	}
 	int offset = TARGET16;
 	u32 targetAddr = GetCompilerPC() + offset + 4;
 
-	BranchInfo branchInfo(GetCompilerPC(), op, GetOffsetInstruction(1), false, likely);
+	MIPSOpcode delaySlotOp = GetOffsetInstruction(1);
 
-	js.downcountAmount += MIPSGetInstructionCycleEstimate(branchInfo.delaySlotOp);
+	js.downcountAmount += MIPSGetInstructionCycleEstimate(delaySlotOp);
 	ir.Write(IROp::VfpuCtrlToReg, IRTEMP_LHS, VFPU_CTRL_CC);
 
-	// 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.
-	if (!likely && !branchInfo.delaySlotIsBranch)
+	if (!likely)
 		CompileDelaySlot();
 
 	int dcAmount = js.downcountAmount;
@@ -318,18 +326,10 @@ void IRFrontend::BranchVFPUFlag(MIPSOpcode op, IRComparison cc, bool likely) {
 
 	ir.Write(IROp::AndConst, IRTEMP_LHS, IRTEMP_LHS, 0, 1 << imm3);
 	FlushAll();
-	ir.Write(ComparisonToExit(cc), 0, IRTEMP_LHS, 0, ResolveNotTakenTarget(branchInfo));
+	ir.Write(ComparisonToExit(cc), 0, IRTEMP_LHS, 0, GetCompilerPC() + 8);
 
-	if (likely && !branchInfo.delaySlotIsBranch)
+	if (likely)
 		CompileDelaySlot();
-	if (branchInfo.delaySlotIsBranch) {
-		// We still link when the branch is taken (targetAddr case.)
-		// Remember, it's from the perspective of the delay slot, so +12.
-		if ((branchInfo.delaySlotInfo & OUT_RA) != 0)
-			ir.WriteSetConstant(MIPS_REG_RA, GetCompilerPC() + 12);
-		if ((branchInfo.delaySlotInfo & OUT_RD) != 0)
-			ir.WriteSetConstant(MIPS_GET_RD(branchInfo.delaySlotOp), GetCompilerPC() + 12);
-	}
 
 	// Taken
 	FlushAll();
@@ -350,10 +350,9 @@ void IRFrontend::Comp_VBranch(MIPSOpcode op) {
 }
 
 void IRFrontend::Comp_Jump(MIPSOpcode op) {
-	if (js.inDelaySlot) {
-		ERROR_LOG_REPORT(Log::JIT, "Branch in Jump delay slot at %08x in block starting at %08x", GetCompilerPC(), js.blockStart);
+	_dbg_assert_(!js.inDelaySlot);
+	if (CompileBranchPair(op))
 		return;
-	}
 
 	u32 off = TARGET26;
 	u32 targetAddr = (GetCompilerPC() & 0xF0000000) | off;
@@ -397,10 +396,9 @@ void IRFrontend::Comp_Jump(MIPSOpcode op) {
 }
 
 void IRFrontend::Comp_JumpReg(MIPSOpcode op) {
-	if (js.inDelaySlot) {
-		ERROR_LOG_REPORT(Log::JIT, "Branch in JumpReg delay slot at %08x in block starting at %08x", GetCompilerPC(), js.blockStart);
+	_dbg_assert_(!js.inDelaySlot);
+	if (CompileBranchPair(op))
 		return;
-	}
 	MIPSGPReg rs = _RS;
 	MIPSGPReg rd = _RD;
 	bool andLink = (op & 0x3f) == 9 && rd != MIPS_REG_ZERO;

Core/MIPS/IR/IRFrontend.h

@@ -140,6 +140,7 @@ private:
 	void BranchRSZeroComp(MIPSOpcode op, IRComparison cc, bool andLink, bool likely);
 	void BranchRSRTComp(MIPSOpcode op, IRComparison cc, bool likely);
 	bool CanContinuePastBranch(const BranchInfo &branchInfo, bool constantOperands);
+	bool CompileBranchPair(MIPSOpcode op);
 
 	// Utilities to reduce duplicated code
 	void CompShiftImm(MIPSOpcode op, IROp shiftType, int sa);

Core/MIPS/Interpreter.cpp

@@ -67,17 +67,54 @@ static inline void DelayBranchTo(MIPSState *mips, u32 where) {
 	mips->inDelaySlot = true;
 }
 
+static inline MIPSOpcode ReadOpAt(const MIPSState *mips, u32 addr) {
+	return MIPSOpcode(mips->space->IsValid4AlignedAddress(addr) ? mips->space->ReadUnchecked_U32(addr) : 0);
+}
+
+// Branches and jumps in a delay slot follow tests/cpu/delayslot/branch. In a jump's (j, jal, jr,
+// jalr) delay slot, a jump or taken branch replaces the jump's target, and a link saves that target.
+// In a conditional branch's, a jump is ignored (but links as usual), and a taken branch replaces
+// the target too.
+static inline bool InJumpDelaySlot(const MIPSState *mips) {
+	return mips->inDelaySlot && (MIPSGetInfo(ReadOpAt(mips, PC - 4)) & IS_JUMP) != 0;
+}
+
+// What a link in the instruction at PC saves.
+static inline u32 LinkAddress(const MIPSState *mips) {
+	return InJumpDelaySlot(mips) ? mips->nextPC : PC + 8;
+}
+
+// A jump in a delay slot. The run loop goes to nextPC next, without another delay slot.
+static inline void DelaySlotJumpTo(MIPSState *mips, u32 where) {
+	if (!InJumpDelaySlot(mips)) {
+		return;
+	}
+	if (!mips->space->IsValidAddress(where) || (where & 3) != 0) {
+		Core_ExecException(where, PC, ExecExceptionType::JUMP);
+	}
+	mips->nextPC = where;
+}
+
 static inline void SkipLikely(MIPSState *mips) {
 	const u32 delaySlotPC = PC + 4;
-	const MIPSOpcode delaySlotOp(mips->space->IsValid4AlignedAddress(delaySlotPC) ? mips->space->ReadUnchecked_U32(delaySlotPC) : 0);
-	// Don't actually skip if it is a jump (seen in Brooktown High.)
-	if (MIPSGetInfo(delaySlotOp) & IS_JUMP) {
-		PC += 4;
-	} else {
-		PC += 8;
-		// The skipped slot costs what it would have run for, as in the IR cores.
-		mips->downcount -= MIPSGetInstructionCycleEstimate(delaySlotOp);
+	const MIPSOpcode delaySlotOp = ReadOpAt(mips, delaySlotPC);
+	// The skipped slot costs what it would have run for, as in the IR cores.
+	mips->downcount -= MIPSGetInstructionCycleEstimate(delaySlotOp);
+	// A skipped jump still jumps, with the instruction after it as its delay slot, but doesn't
+	// link (tests/cpu/delayslot/branch, seen in Brooktown High). Not when this is itself in a delay
+	// slot, which is over after it.
+	if ((MIPSGetInfo(delaySlotOp) & IS_JUMP) && !mips->inDelaySlot) {
+		u32 target;
+		if ((delaySlotOp >> 26) == 0) {
+			target = R((delaySlotOp >> 21) & 0x1F) & ~3;  // jr, jalr
+		} else {
+			target = (delaySlotPC & 0xF0000000) | ((delaySlotOp & 0x03FFFFFF) << 2);  // j, jal
+		}
+		PC = delaySlotPC;
+		DelayBranchTo(mips, target);
+		return;
 	}
+	PC += 8;
 }
 
 int MIPS_InterpretSingleStep(MIPSState *mips) {
@@ -259,10 +296,10 @@ namespace MIPSInt {
 		case 1: if (value >= 0) DelayBranchTo(mips, addr); else PC += 4; break;//bgez
 		case 2: if (value <  0) DelayBranchTo(mips, addr); else SkipLikely(mips); break;//bltzl
 		case 3: if (value >= 0) DelayBranchTo(mips, addr); else SkipLikely(mips); break;//bgezl
-		case 16: R(MIPS_REG_RA) = PC + 8; if (value <  0) DelayBranchTo(mips, addr); else PC += 4; break;//bltzal
-		case 17: R(MIPS_REG_RA) = PC + 8; if (value >= 0) DelayBranchTo(mips, addr); else PC += 4; break;//bgezal
-		case 18: R(MIPS_REG_RA) = PC + 8; if (value <  0) DelayBranchTo(mips, addr); else SkipLikely(mips); break;//bltzall
-		case 19: R(MIPS_REG_RA) = PC + 8; if (value >= 0) DelayBranchTo(mips, addr); else SkipLikely(mips); break;//bgezall
+		case 16: R(MIPS_REG_RA) = LinkAddress(mips); if (value <  0) DelayBranchTo(mips, addr); else PC += 4; break;//bltzal
+		case 17: R(MIPS_REG_RA) = LinkAddress(mips); if (value >= 0) DelayBranchTo(mips, addr); else PC += 4; break;//bgezal
+		case 18: R(MIPS_REG_RA) = LinkAddress(mips); if (value <  0) DelayBranchTo(mips, addr); else SkipLikely(mips); break;//bltzall
+		case 19: R(MIPS_REG_RA) = LinkAddress(mips); if (value >= 0) DelayBranchTo(mips, addr); else SkipLikely(mips); break;//bgezall
 		default:
 			_dbg_assert_msg_(false,"Trying to interpret instruction that can't be interpreted");
 			break;
@@ -301,36 +338,27 @@ namespace MIPSInt {
 	}
 
 	void Int_JumpType(MIPSState *mips, MIPSOpcode op) {
-		if (mips->inDelaySlot) {
-			// There's one of these in Star Soldier at 0881808c, which seems benign.
-			ERROR_LOG(Log::CPU, "Jump in delay slot at %08x", mips->pc);
-		}
-
 		const u32 off = ((op & 0x03FFFFFF) << 2);
 		const u32 addr = (mips->pc & 0xF0000000) | off;
 
 		switch (op >> 26) {
 		case 2: //j
-			if (!mips->inDelaySlot)
-				DelayBranchTo(mips, addr);
 			break;
 		case 3: //jal
-			R(MIPS_REG_RA) = PC + 8;
-			if (!mips->inDelaySlot)
-				DelayBranchTo(mips, addr);
+			R(MIPS_REG_RA) = LinkAddress(mips);
 			break;
 		default:
 			_dbg_assert_msg_(false,"Trying to interpret instruction that can't be interpreted");
-			break;
+			return;
+		}
+		if (mips->inDelaySlot) {
+			DelaySlotJumpTo(mips, addr);
+		} else {
+			DelayBranchTo(mips, addr);
 		}
 	}
 
 	void Int_JumpRegType(MIPSState *mips, MIPSOpcode op) {
-		if (mips->inDelaySlot) {
-			// There's one of these in Star Soldier at 0881808c, which seems benign.
-			ERROR_LOG(Log::CPU, "Jump in delay slot at %08x", mips->pc);
-		}
-
 		int rs = _RS;
 		int rd = _RD;
 		// The CPUs ignore the target's low two bits (tests/cpu/jump/unaligned). Sony's ME firmware keeps
@@ -338,16 +366,18 @@ namespace MIPSInt {
 		u32 addr = R(rs) & ~3;
 		switch (op & 0x3f) {
 		case 8: //jr
-			if (!mips->inDelaySlot)
-				DelayBranchTo(mips, addr);
 			break;
 		case 9: //jalr
 			if (rd != 0)
-				R(rd) = PC + 8;
-			// Update rd, but otherwise do not take the branch if we're branching.
-			if (!mips->inDelaySlot)
-				DelayBranchTo(mips, addr);
+				R(rd) = LinkAddress(mips);
 			break;
+		default:
+			return;
+		}
+		if (mips->inDelaySlot) {
+			DelaySlotJumpTo(mips, addr);
+		} else {
+			DelayBranchTo(mips, addr);
 		}
 	}
 
@@ -1350,4 +1380,29 @@ namespace MIPSInt {
 			MIPSInterpret(mips, Memory::Read_Instruction(PC));
 		}
 	}
+
+	// Runs the branch at PC until it's out of its delay slot, charging as the run loop does. The IR
+	// frontend leaves a branch with a branch or jump in its delay slot to this.
+	void Int_BranchPair(MIPSState *mips, MIPSOpcode op) {
+		// Not where the IR frontend puts it: the game is executing data.
+		if (mips->inDelaySlot || !(MIPSGetInfo(ReadOpAt(mips, PC)) & DELAYSLOT)) {
+			Core_ExecException(PC, PC, ExecExceptionType::ILLEGAL);
+			return;
+		}
+		do {
+			if (!mips->space->IsValid4AlignedAddress(PC)) {
+				Core_ExecException(PC, PC, ExecExceptionType::JUMP);
+				return;
+			}
+			const MIPSOpcode cur = ReadExecutedOp(*mips->space, PC);
+			const bool wasInDelaySlot = mips->inDelaySlot;
+			MIPSInterpret(mips, cur);
+			mips->downcount -= MIPSGetInstructionCycleEstimate(cur);
+			// A syscall in the delay slot leaves it itself.
+			if (mips->inDelaySlot && wasInDelaySlot) {
+				PC = mips->nextPC;
+				mips->inDelaySlot = false;
+			}
+		} while (mips->inDelaySlot);
+	}
 }

Core/MIPS/Interpreter.h

@@ -47,6 +47,7 @@ namespace MIPSInt
 	void Int_FPUComp(MIPSState *mips, MIPSOpcode op);
 	void Int_FPUBranch(MIPSState *mips, MIPSOpcode op);
 	void Int_CallReplacement(MIPSState *mips, MIPSOpcode op);
+	void Int_BranchPair(MIPSState *mips, MIPSOpcode op);
 	void Int_Special2(MIPSState *mips, MIPSOpcode op);
 	void Int_Halt(MIPSState *mips, MIPSOpcode op);
 	void Int_Me2C(MIPSState *mips, MIPSOpcode op);

Core/MIPS/InterpreterDispatch.cpp

@@ -352,6 +352,10 @@ int ExecInstruction(MIPSState *mips, MIPSOpcode op) {
     case 26:
     {
         switch ((op.encoding >> 24) & 0x3) {
+        // BranchPair
+        case 1:
+            MIPSInt::Int_BranchPair(mips, op);
+            return 1;
         // CallRepl
         case 2:
             MIPSInt::Int_CallReplacement(mips, op);

Core/MIPS/JitCommon/JitCommon.cpp

@@ -75,30 +75,8 @@ namespace MIPSComp {
 
 	BranchInfo::BranchInfo(u32 pc, MIPSOpcode o, MIPSOpcode delayO, bool al, bool l)
 		: compilerPC(pc), op(o), delaySlotOp(delayO), likely(l), andLink(al) {
-		delaySlotInfo = MIPSGetInfo(delaySlotOp).value;
-		delaySlotIsBranch = (delaySlotInfo & (IS_JUMP | IS_CONDBRANCH)) != 0;
 	}
 
-	u32 ResolveNotTakenTarget(const BranchInfo &branchInfo) {
-		u32 notTakenTarget = branchInfo.compilerPC + 8;
-		if ((branchInfo.delaySlotInfo & (IS_JUMP | IS_CONDBRANCH)) != 0) {
-			// If a branch has a j/jr/jal/jalr as a delay slot, that is run if the branch is not taken.
-			// TODO: Technically, in the likely case, we should somehow suppress andLink on this exit.
-			bool isJump = (branchInfo.delaySlotInfo & IS_JUMP) != 0;
-			// If the delay slot is a branch, likely skips it.
-			if (isJump || !branchInfo.likely)
-				notTakenTarget -= 4;
-
-			// For a branch (not a jump), it actually should try the delay slot and take its target potentially.
-			// This is similar to the VFPU case and has not been seen, so just report it.
-			if (!isJump && SignExtend16ToU32(branchInfo.delaySlotOp) != SignExtend16ToU32(branchInfo.op) - 1)
-				ERROR_LOG_REPORT(Log::JIT, "Branch in branch delay slot at %08x with different target", branchInfo.compilerPC);
-			if (isJump && branchInfo.likely && (branchInfo.delaySlotInfo & (OUT_RA | OUT_RD)) != 0)
-				ERROR_LOG_REPORT(Log::JIT, "Jump in likely branch delay slot with link at %08x", branchInfo.compilerPC);
-	}
-		return notTakenTarget;
-}
-
 	bool NativeJitRunsMediaEngine() {
 #if PPSSPP_ARCH(ARM64) || PPSSPP_ARCH(AMD64)
 		return true;

Core/MIPS/JitCommon/JitCommon.h

@@ -176,17 +176,12 @@ namespace MIPSComp {
 		u32 compilerPC;
 		MIPSOpcode op;
 		MIPSOpcode delaySlotOp;
-		u64 delaySlotInfo;
 		bool likely;
 		bool andLink;
 		// Update manually if it's not always nice (rs/rt, rs/zero, etc.)
 		bool delaySlotIsNice = true;
-		bool delaySlotIsBranch;
 	};
 
-	// This seems to be the same for all branch types.
-	u32 ResolveNotTakenTarget(const BranchInfo &branchInfo);
-
 	void DoDummyJitState(PointerWrap &p, const MIPSState *mips);
 
 	JitInterface *CreateNativeJit(MIPSState *mipsState);

Core/MIPS/MIPS.h

@@ -42,6 +42,9 @@ typedef Memory::Opcode MIPSOpcode;
 // 26, which the Allegrex doesn't have, decodes it as CallRepl.
 #define MIPS_CALL_REPLACEMENT_OP 0x6A000000
 #define MIPS_REPLACEMENT_INDEX_MASK 0x00FFFFFF
+// Another (BranchPair): the IR frontend's way to run a branch with a branch or jump in its delay
+// slot, which it leaves to the interpreter. Also never in memory.
+#define MIPS_BRANCH_PAIR_OP 0x69000000
 
 enum MIPSGPReg {
 	MIPS_REG_ZERO=0,

Core/MIPS/MIPSTables.cpp

@@ -820,7 +820,7 @@ static const MIPSInstruction tableALLEGREX0[32] =  // 011111 ..... ..... ..... x
 
 static const MIPSInstruction tableEMU[4] = {
 	INVALID,
-	INVALID,
+	INSTR("BranchPair", JITFUNC(Comp_Generic), Dis_Generic, Int_BranchPair, 0),
 	INSTR("CallRepl", JITFUNC(Comp_ReplacementFunc), Dis_CallReplacement, Int_CallReplacement, 0),
 	INVALID,
 };
@@ -1239,7 +1239,7 @@ static inline bool StepIRBlock(IRBlockState &b, const MIPSState *mips, MIPSOpcod
 		return false;
 	}
 	if (mips->inDelaySlot) {
-		// Taken, or a jump.
+		// Taken, or a jump (or one a likely branch skipped).
 		b.endAfterNext = true;
 		return false;
 	}
@@ -1247,9 +1247,13 @@ static inline bool StepIRBlock(IRBlockState &b, const MIPSState *mips, MIPSOpcod
 		// A likely branch that wasn't taken skipped its delay slot.
 		return true;
 	}
-	// Not taken: the delay slot runs next, as an ordinary instruction.
+	// Not taken: the delay slot runs next, as an ordinary instruction. If it's a branch or jump, the
+	// IR left the two to the interpreter (CompileBranchPair), and the block ends here.
 	const MIPSInfo delaySlotInfo = MIPSGetInfo(ReadDelaySlot(mips, pc));
-	if (IR_CONTINUE_BRANCHES && IsContinuableBranch(op) && (delaySlotInfo & (IS_JUMP | IS_CONDBRANCH)) == 0) {
+	if ((delaySlotInfo & (IS_JUMP | IS_CONDBRANCH)) != 0) {
+		return true;
+	}
+	if (IR_CONTINUE_BRANCHES && IsContinuableBranch(op)) {
 		b.continueAfterNext = true;
 	} else {
 		b.endAfterNext = true;

docs/cpu-equivalence.md

@@ -25,7 +25,7 @@ Environment variables:
 |---|---|
 | `PPSSPP_CPUFUZZ_COUNT` | number of cases (default 2000) |
 | `PPSSPP_CPUFUZZ_SEED` | seed of the first case; case i uses seed + i |
-| `PPSSPP_CPUFUZZ_CLASSES` | weighted string of `a` (ALU), `i` (immediate), `f` (FPU), `v` (VFPU), `m` (memory), `r` (a run of vdots the IR batches into one `Vec4VfpuMultiDot`); `b` anywhere adds forward conditional branches |
+| `PPSSPP_CPUFUZZ_CLASSES` | weighted string of `a` (ALU), `i` (immediate), `f` (FPU), `v` (VFPU), `m` (memory), `r` (a run of vdots the IR batches into one `Vec4VfpuMultiDot`); `b` anywhere adds forward conditional branches; `d` adds those, `j`/`jal`, the likely links and the FPU and VFPU branches, sometimes in each other's delay slots |
 | `PPSSPP_CPUFUZZ_CODE` | comma-separated hex instructions to run instead of the generated code |
 | `PPSSPP_CPUFUZZ_CODE_SEED` | apply `PPSSPP_CPUFUZZ_CODE` to this seed only |
 | `PPSSPP_CPUFUZZ_SKIP` | cores to skip, by digit: 1 IR interpreter, 2 JIT |
@@ -126,6 +126,9 @@ IR-based (the "IR JIT" and "native IR backend" entries).
 - The IR frontend continues past conditional branches, so a block can now leave with a prefix
   pending at a mid-block exit. It doesn't continue then, as only the end of a block checks that.
 - Hardware answers 1, 2 (host agreement), 5, 7 (the interpreter and `vcmov`), 8, 9 and 10 below.
+- Branches and jumps in delay slots, which neither core ran as the hardware does
+  (`tests/cpu/delayslot/branch`). The IR frontend now leaves such a pair to the interpreter
+  (`CompileBranchPair()`, the `BranchPair` pseudo-op), so the cores share one implementation.
 
 Timing, which decides when interrupts, thread switches and other events happen, so a difference
 sends a game down a different path on each core:

test.py

@@ -96,6 +96,7 @@ tests_good = [
   "cpu/cpu_alu/shift_encodings",
   "cpu/cpu_alu/allegrex_encodings",
   "cpu/jump/unaligned",
+  "cpu/delayslot/branch",
   "cpu/vfpu/callout",
   "cpu/vfpu/colors",
   "cpu/vfpu/convert",

unittest/JitHarness.cpp

@@ -1096,7 +1096,7 @@ bool TestMeDmacplus() {
 //                           runs on the instructions being worked on. "dot" is the dot products
 //                           and the prefixes. Without PPSSPP_CPUFUZZ_CLASSES, the class is "v"
 //                           ("vr" for "dot": r is a run of vdots the IR batches). b adds forward
-//                           conditional branches.
+//                           conditional branches, d those and jumps, also in delay slots.
 
 namespace {
 
@@ -1302,29 +1302,47 @@ struct FuzzCase {
 	FuzzState init;
 };
 
+// A branch at index from to index target. With jumps, also j/jal, likely links and the FPU and VFPU
+// branches.
+static u32 MakeFuzzBranch(FuzzRng &rng, int from, int target, bool jumps) {
+	const u32 offset = (u32)(target - (from + 1)) & 0xFFFF;
+	const u32 rs = 1 + rng.Below(31), rt = rng.Chance(30) ? 0 : 1 + rng.Below(31);
+	switch (rng.Below(jumps ? 10 : 6)) {
+	case 0: return (0x04 << 26) | (rs << 21) | (rt << 16) | offset;  // beq
+	case 1: return (0x05 << 26) | (rs << 21) | (rt << 16) | offset;  // bne
+	case 2: return ((rng.Chance(50) ? 0x06 : 0x07) << 26) | (rs << 21) | offset;  // blez, bgtz
+	case 3: return (0x01 << 26) | (rs << 21) | (rng.Below(2) << 16) | offset;  // bltz, bgez
+	case 4: return ((rng.Chance(50) ? 0x14 : 0x15) << 26) | (rs << 21) | (rt << 16) | offset;  // beql, bnel
+	case 5: return (0x01 << 26) | (rs << 21) | ((rng.Chance(50) ? 0x10 : 0x11) << 16) | offset;  // bltzal, bgezal
+	case 6: return (0x01 << 26) | (rs << 21) | ((rng.Chance(50) ? 0x12 : 0x13) << 16) | offset;  // bltzall, bgezall
+	case 7: return 0x45000000 | (rng.Below(4) << 16) | offset;  // bc1f, bc1t, bc1fl, bc1tl
+	case 8: return 0x49000000 | (rng.Below(8) << 18) | (rng.Below(4) << 16) | offset;  // bvf, bvt, bvfl, bvtl
+	default: {
+		const u32 addr = PSP_GetUserMemoryBase() + FUZZ_CODE_OFFSET + target * 4;
+		return ((rng.Chance(50) ? 0x02 : 0x03) << 26) | ((addr >> 2) & 0x03FFFFFF);  // j, jal
+	}
+	}
+}
+
 // Turns some instructions into conditional branches forward, to a later instruction or the end, so
-// every case still finishes. Never two in a row, so each delay slot is an ordinary instruction.
-static void AddFuzzBranches(FuzzRng &rng, std::vector<u32> &code) {
+// every case still finishes. Without delaySlots, never two in a row, so each delay slot is an
+// ordinary instruction. With it, the delay slot is sometimes a branch or jump too, and jumps are in.
+static void AddFuzzBranches(FuzzRng &rng, std::vector<u32> &code, bool delaySlots) {
 	const int n = (int)code.size();
 	for (int i = 0; i + 1 < n; i++) {
 		if (!rng.Chance(20))
 			continue;
 		const int target = i + 2 + (int)rng.Below(n - i - 1);  // up to n, the terminator
-		const u32 offset = (u32)(target - (i + 1)) & 0xFFFF;
-		const u32 rs = 1 + rng.Below(31), rt = rng.Chance(30) ? 0 : 1 + rng.Below(31);
-		u32 op;
-		switch (rng.Below(6)) {
-		case 0: op = (0x04 << 26) | (rs << 21) | (rt << 16); break;  // beq
-		case 1: op = (0x05 << 26) | (rs << 21) | (rt << 16); break;  // bne
-		case 2: op = ((rng.Chance(50) ? 0x06 : 0x07) << 26) | (rs << 21); break;  // blez, bgtz
-		case 3: op = (0x01 << 26) | (rs << 21) | (rng.Below(2) << 16); break;  // bltz, bgez
-		case 4: op = ((rng.Chance(50) ? 0x14 : 0x15) << 26) | (rs << 21) | (rt << 16); break;  // beql, bnel
-		default: op = (0x01 << 26) | (rs << 21) | ((rng.Chance(50) ? 0x10 : 0x11) << 16); break;  // bltzal, bgezal
-		}
-		code[i] = op | offset;
-		// The delay slot mustn't be a branch.
-		if (MIPSGetInfo(MIPSOpcode(code[i + 1])) & (IS_CONDBRANCH | IS_JUMP))
+		code[i] = MakeFuzzBranch(rng, i, target, delaySlots);
+		// The inner one's own delay slot (i + 2) has to be before the terminator.
+		if (delaySlots && i + 2 < n && rng.Chance(50)) {
+			const int innerTarget = i + 3 + (int)rng.Below(n - i - 2);
+			code[i + 1] = MakeFuzzBranch(rng, i + 1, innerTarget, true);
+			// Its delay slot stays an ordinary instruction.
+			i++;
+		} else if (MIPSGetInfo(MIPSOpcode(code[i + 1])) & (IS_CONDBRANCH | IS_JUMP)) {
 			code[i + 1] = 0;
+		}
 		i++;
 	}
 }
@@ -1429,8 +1447,9 @@ static FuzzCase GenerateFuzzCase(u64 seed, const char *classes, const std::vecto
 		else
 			c.code.push_back(GenerateFuzzOp(rng, classes));
 	}
-	if (strchr(classes, 'b'))
-		AddFuzzBranches(rng, c.code);
+	const bool delaySlots = strchr(classes, 'd') != nullptr;
+	if (strchr(classes, 'b') || delaySlots)
+		AddFuzzBranches(rng, c.code, delaySlots);
 	if (!codeOverride.empty()) {
 		c.code = codeOverride;
 	}
@@ -1819,9 +1838,11 @@ bool TestCPUEquivalence() {
 	};
 	const int numCores = (int)ARRAY_SIZE(cores);
 
+	bool skipped[ARRAY_SIZE(cores)]{};
 	std::vector<FuzzState> results((size_t)count * numCores);
 	for (int c = 0; c < numCores; ++c) {
-		if (c > 0 && skipCores && strchr(skipCores, '0' + c)) {
+		skipped[c] = c > 0 && skipCores && strchr(skipCores, '0' + c);
+		if (skipped[c]) {
 			for (int i = 0; i < count; ++i) {
 				results[(size_t)i * numCores + c] = results[(size_t)i * numCores];
 			}
@@ -1905,6 +1926,10 @@ bool TestCPUEquivalence() {
 
 	bool success = true;
 	for (int c = 1; c < numCores; ++c) {
+		if (skipped[c]) {
+			printf("%s: skipped\n", CoreName(cores[c]));
+			continue;
+		}
 		printf("%s: %d of %d cases differ from the interpreter\n", CoreName(cores[c]), failures[c], count);
 		if (failures[c]) {
 			success = false;