diff --git a/Core/MIPS/IR/IRCompBranch.cpp b/Core/MIPS/IR/IRCompBranch.cpp index 8077107d19..4827d1a92a 100644 --- a/Core/MIPS/IR/IRCompBranch.cpp +++ b/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; diff --git a/Core/MIPS/IR/IRFrontend.h b/Core/MIPS/IR/IRFrontend.h index a363900c8b..f69b127a08 100644 --- a/Core/MIPS/IR/IRFrontend.h +++ b/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); diff --git a/Core/MIPS/Interpreter.cpp b/Core/MIPS/Interpreter.cpp index 84f0ee9ec8..595bec640c 100644 --- a/Core/MIPS/Interpreter.cpp +++ b/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); + } } diff --git a/Core/MIPS/Interpreter.h b/Core/MIPS/Interpreter.h index e89b633a28..f1dc133fdf 100644 --- a/Core/MIPS/Interpreter.h +++ b/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); diff --git a/Core/MIPS/InterpreterDispatch.cpp b/Core/MIPS/InterpreterDispatch.cpp index 6d0aded843..eba43fb735 100644 --- a/Core/MIPS/InterpreterDispatch.cpp +++ b/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); diff --git a/Core/MIPS/JitCommon/JitCommon.cpp b/Core/MIPS/JitCommon/JitCommon.cpp index 1361480ec4..6ddd4ae5cc 100644 --- a/Core/MIPS/JitCommon/JitCommon.cpp +++ b/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; diff --git a/Core/MIPS/JitCommon/JitCommon.h b/Core/MIPS/JitCommon/JitCommon.h index 7bee89689c..6235c4a790 100644 --- a/Core/MIPS/JitCommon/JitCommon.h +++ b/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); diff --git a/Core/MIPS/MIPS.h b/Core/MIPS/MIPS.h index 36e7cea261..3301e77461 100644 --- a/Core/MIPS/MIPS.h +++ b/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, diff --git a/Core/MIPS/MIPSTables.cpp b/Core/MIPS/MIPSTables.cpp index 9d2a679146..071fdaf218 100644 --- a/Core/MIPS/MIPSTables.cpp +++ b/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; diff --git a/docs/cpu-equivalence.md b/docs/cpu-equivalence.md index 9b6fc793f6..d0530d86ea 100644 --- a/docs/cpu-equivalence.md +++ b/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: diff --git a/pspautotests b/pspautotests index 26a3dd0155..fcd60cb905 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 26a3dd0155d385e96430929b9d40cfeb4f320570 +Subproject commit fcd60cb9055c233f50f20e010e65a5ff30ac8539 diff --git a/test.py b/test.py index e200885785..5a4c567cf5 100755 --- a/test.py +++ b/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", diff --git a/unittest/JitHarness.cpp b/unittest/JitHarness.cpp index df1e684c33..6e304e3079 100644 --- a/unittest/JitHarness.cpp +++ b/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 &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 &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 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;