From cf869fc24f47ea4b86ce7634c0aab08e4c115e69 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Sat, 13 Sep 2014 20:37:06 +1200 Subject: [PATCH] Fix Idle Skipping in JitIL. Has been broken since the flags-opt merge. The idle skipping code in JitIL was very brittle and depended on the IL of it's inputs not changing in any way. flags-opt changed the IR generated by the cmp instruction, which is part of the idle loop, causing JitIL to break in really weird ways, which were almost impossible to track down. This fixes various wii games crashing/not booting and the Regspill error on (all?) gamecube mmu games. --- Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp | 13 +++++++------ Source/Core/Core/PowerPC/JitILCommon/IR.cpp | 7 ------- .../PowerPC/JitILCommon/JitILBase_Branch.cpp | 16 ++++++++++------ .../PowerPC/JitILCommon/JitILBase_LoadStore.cpp | 16 ++++++++++++++++ 4 files changed, 33 insertions(+), 19 deletions(-) diff --git a/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp b/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp index 695dab795e..4a5a43313b 100644 --- a/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp +++ b/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp @@ -909,7 +909,7 @@ static void DoWriteCode(IRBuilder* ibuild, JitIL* Jit, u32 exitAddress) regMarkUse(RI, I, getOp1(I), 1); break; case IdleBranch: - regMarkUse(RI, I, getOp1(getOp1(I)), 1); + regMarkUse(RI, I, getOp1(I), 1); break; case BranchCond: { @@ -2088,9 +2088,10 @@ static void DoWriteCode(IRBuilder* ibuild, JitIL* Jit, u32 exitAddress) case IdleBranch: { - Jit->CMP(32, regLocForInst(RI, getOp1(getOp1(I))), - Imm32(RI.Build->GetImmValue(getOp2(getOp1(I))))); - FixupBranch cont = Jit->J_CC(CC_NE); + // If value is 0, we don't need to call out to the idle function. + OpArg value = regLocForInst(RI, getOp1(I)); + Jit->TEST(32, value, value); + FixupBranch noidle = Jit->J_CC(CC_NZ); RI.Jit->Cleanup(); // is it needed? Jit->ABI_CallFunction((void *)&PowerPC::OnIdleIL); @@ -2098,9 +2099,9 @@ static void DoWriteCode(IRBuilder* ibuild, JitIL* Jit, u32 exitAddress) Jit->MOV(32, PPCSTATE(pc), Imm32(ibuild->GetImmValue( getOp2(I) ))); Jit->WriteExceptionExit(); - Jit->SetJumpTarget(cont); + Jit->SetJumpTarget(noidle); if (RI.IInfo[I - RI.FirstI] & 4) - regClearInst(RI, getOp1(getOp1(I))); + regClearInst(RI, getOp1(I)); if (RI.IInfo[I - RI.FirstI] & 8) regClearInst(RI, getOp2(I)); break; diff --git a/Source/Core/Core/PowerPC/JitILCommon/IR.cpp b/Source/Core/Core/PowerPC/JitILCommon/IR.cpp index c07e1a1216..789bcc09de 100644 --- a/Source/Core/Core/PowerPC/JitILCommon/IR.cpp +++ b/Source/Core/Core/PowerPC/JitILCommon/IR.cpp @@ -1083,11 +1083,6 @@ InstLoc IRBuilder::FoldBranchCond(InstLoc Op1, InstLoc Op2) return EmitBiOp(BranchCond, Op1, Op2); } -InstLoc IRBuilder::FoldIdleBranch(InstLoc Op1, InstLoc Op2) -{ - return EmitBiOp(IdleBranch, EmitICmpEq(getOp1(getOp1(Op1)), getOp2(getOp1(Op1))), Op2); -} - InstLoc IRBuilder::FoldICmp(unsigned Opcode, InstLoc Op1, InstLoc Op2) { if (isImm(*Op1)) @@ -1245,8 +1240,6 @@ InstLoc IRBuilder::FoldBiOp(unsigned Opcode, InstLoc Op1, InstLoc Op2, unsigned return FoldRol(Op1, Op2); case BranchCond: return FoldBranchCond(Op1, Op2); - case IdleBranch: - return FoldIdleBranch(Op1, Op2); case ICmpEq: case ICmpNe: case ICmpUgt: case ICmpUlt: case ICmpUge: case ICmpUle: case ICmpSgt: case ICmpSlt: case ICmpSge: case ICmpSle: diff --git a/Source/Core/Core/PowerPC/JitILCommon/JitILBase_Branch.cpp b/Source/Core/Core/PowerPC/JitILCommon/JitILBase_Branch.cpp index 743e5d1afd..88a2eb3cc3 100644 --- a/Source/Core/Core/PowerPC/JitILCommon/JitILBase_Branch.cpp +++ b/Source/Core/Core/PowerPC/JitILCommon/JitILBase_Branch.cpp @@ -138,14 +138,18 @@ void JitILBase::bcx(UGeckoInstruction inst) else destination = js.compilerPC + SignExt16(inst.BD << 2); + // Idle skipping: + // The main Idle skipping is done in the LoadStore code, but there is an optimization here. + // If idle skipping is enabled, then this branch will only be reached when the branch is not + // taken. if (SConfig::GetInstance().m_LocalCoreStartupParameter.bSkipIdle && - inst.hex == 0x4182fff8 && - (Memory::ReadUnchecked_U32(js.compilerPC - 8) & 0xFFFF0000) == 0x800D0000 && - (Memory::ReadUnchecked_U32(js.compilerPC - 4) == 0x28000000 || - (SConfig::GetInstance().m_LocalCoreStartupParameter.bWii && Memory::ReadUnchecked_U32(js.compilerPC - 4) == 0x2C000000)) - ) + inst.hex == 0x4182fff8 && + (Memory::ReadUnchecked_U32(js.compilerPC - 8) & 0xFFFF0000) == 0x800D0000 && + (Memory::ReadUnchecked_U32(js.compilerPC - 4) == 0x28000000 || + (SConfig::GetInstance().m_LocalCoreStartupParameter.bWii && Memory::ReadUnchecked_U32(js.compilerPC - 4) == 0x2C000000)) + ) { - ibuild.EmitIdleBranch(Test, ibuild.EmitIntConst(destination)); + // Uh, Do nothing. } else { diff --git a/Source/Core/Core/PowerPC/JitILCommon/JitILBase_LoadStore.cpp b/Source/Core/Core/PowerPC/JitILCommon/JitILBase_LoadStore.cpp index 0147fa61e3..0e20e79f8c 100644 --- a/Source/Core/Core/PowerPC/JitILCommon/JitILBase_LoadStore.cpp +++ b/Source/Core/Core/PowerPC/JitILCommon/JitILBase_LoadStore.cpp @@ -33,6 +33,22 @@ void JitILBase::lXz(UGeckoInstruction inst) ibuild.EmitStoreGReg(addr, inst.RA); IREmitter::InstLoc val; + + // Idle Skipping. This really should be done somewhere else. + // Either lower in the IR or higher in PPCAnalyist + if (SConfig::GetInstance().m_LocalCoreStartupParameter.bSkipIdle && + inst.OPCD == 32 && // Lwx + (inst.hex & 0xFFFF0000) == 0x800D0000 && + (Memory::ReadUnchecked_U32(js.compilerPC + 4) == 0x28000000 || + (SConfig::GetInstance().m_LocalCoreStartupParameter.bWii && Memory::ReadUnchecked_U32(js.compilerPC + 4) == 0x2C000000)) && + Memory::ReadUnchecked_U32(js.compilerPC + 8) == 0x4182fff8) + { + val = ibuild.EmitLoad32(addr); + ibuild.EmitIdleBranch(val, ibuild.EmitIntConst(js.compilerPC)); + ibuild.EmitStoreGReg(val, inst.RD); + return; + } + switch (inst.OPCD & ~0x1) { case 32: // lwz