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.

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