From e9459fb30a66a694b2012ea55b5f56e0d5612489 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Sun, 5 Apr 2015 20:17:50 +1200 Subject: [PATCH 1/2] JitIL: Fix a bug in floatpoint load/store instructions. The regBuildMemAddress function already clears the address register. Not only is clearing it again pointless, regBuildMemAddress uses the bits in IInfo slightly diffrently and the second clear can clear the wrong registers causing bugs if something else actually needs to use those registers. --- Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp b/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp index a9164b0bb8..ff8b7d6cc3 100644 --- a/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp +++ b/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp @@ -1563,7 +1563,6 @@ static void DoWriteCode(IRBuilder* ibuild, JitIL* Jit, u32 exitAddress) RI.Jit->SafeLoadToReg(RSCRATCH2, info.first, 32, info.second, regsInUse(RI), false); Jit->MOVD_xmm(reg, R(RSCRATCH2)); RI.fregs[reg] = I; - regNormalRegClear(RI, I); break; } case LoadDouble: @@ -1577,7 +1576,6 @@ static void DoWriteCode(IRBuilder* ibuild, JitIL* Jit, u32 exitAddress) RI.Jit->SafeLoadToReg(RSCRATCH2, info.first, 64, info.second, regsInUse(RI), false); Jit->MOVQ_xmm(reg, R(RSCRATCH2)); RI.fregs[reg] = I; - regNormalRegClear(RI, I); break; } case LoadPaired: @@ -1624,8 +1622,6 @@ static void DoWriteCode(IRBuilder* ibuild, JitIL* Jit, u32 exitAddress) if (RI.IInfo[I - RI.FirstI] & 4) fregClearInst(RI, getOp1(I)); - if (RI.IInfo[I - RI.FirstI] & 8) - regClearInst(RI, getOp2(I)); break; } case StoreDouble: @@ -1646,8 +1642,6 @@ static void DoWriteCode(IRBuilder* ibuild, JitIL* Jit, u32 exitAddress) if (RI.IInfo[I - RI.FirstI] & 4) fregClearInst(RI, getOp1(I)); - if (RI.IInfo[I - RI.FirstI] & 8) - regClearInst(RI, getOp2(I)); break; } case StorePaired: From 9fdc713c87c11dc6356727b87035a2c9fc9b0d39 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Sun, 5 Apr 2015 21:18:05 +1200 Subject: [PATCH 2/2] While I'm here, Add some comments. --- Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp b/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp index ff8b7d6cc3..5c95aa39b2 100644 --- a/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp +++ b/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp @@ -43,8 +43,19 @@ struct RegInfo JitIL *Jit; IRBuilder* Build; InstLoc FirstI; + + // IInfo contains (per instruction) + // Bits 0-1: Saturating count of number of instructions referencing this instruction. + // Bits 2-3: single bit per operand marking if this is the last instruction to reference that operand's result. + // Used to decide if we should free any registers associated with the operands after this instruction + // and if we can clobber the operands registers. + // Warning, Memory instruction use these bits slightly differently. + // Bits 15-31: Spill location std::vector IInfo; + + // The last instruction which uses the result of this instruction. Used by the register allocator. std::vector lastUsed; + InstLoc regs[MAX_NUMBER_OF_REGS]; InstLoc fregs[MAX_NUMBER_OF_REGS]; unsigned numSpills; @@ -412,6 +423,8 @@ static X64Reg regBinLHSReg(RegInfo& RI, InstLoc I) return reg; } +// Clear any registers which end their lifetime at I +// Don't use this for special instructions like memory load/stores static void regNormalRegClear(RegInfo& RI, InstLoc I) { if (RI.IInfo[I - RI.FirstI] & 4) @@ -420,6 +433,7 @@ static void regNormalRegClear(RegInfo& RI, InstLoc I) regClearInst(RI, getOp2(I)); } +// Clear any floating point registers which end their lifetime at I static void fregNormalRegClear(RegInfo& RI, InstLoc I) { if (RI.IInfo[I - RI.FirstI] & 4)