From 89c4068663ca961cd9cec40c94eac4bf0f2d6583 Mon Sep 17 00:00:00 2001 From: "j4ck.fr0st" Date: Wed, 30 Jun 2010 16:17:20 +0000 Subject: [PATCH] Fix a bug in DSP Jit where branches had a blockSize of zero. Add some useful checking to J_CC/SetJumpTarget. Refactor increment/decrement and reuse code with increase/decrease. git-svn-id: https://dolphin-emu.googlecode.com/svn/trunk@5816 8ced0084-cf51-0410-be5f-012b33b47a6e --- Source/Core/Common/Src/x64Emitter.cpp | 7 +- Source/Core/DSPCore/Src/DSPEmitter.cpp | 11 +- Source/Core/DSPCore/Src/DSPEmitter.h | 2 + Source/Core/DSPCore/Src/Jit/DSPJitUtil.cpp | 198 ++++++++++----------- 4 files changed, 109 insertions(+), 109 deletions(-) diff --git a/Source/Core/Common/Src/x64Emitter.cpp b/Source/Core/Common/Src/x64Emitter.cpp index 996d51ae74..a916de02c7 100644 --- a/Source/Core/Common/Src/x64Emitter.cpp +++ b/Source/Core/Common/Src/x64Emitter.cpp @@ -389,7 +389,8 @@ void XEmitter::J_CC(CCFlags conditionCode, const u8 * addr, bool force5Bytes) u64 fn = (u64)addr; if (!force5Bytes) { - s32 distance = (s32)(fn - ((u64)code + 2)); //TODO - sanity check + s32 distance = (s32)(fn - ((u64)code + 2)); + _assert_msg_(DYNA_REC, distance < 0x80, "Jump target too far away, needs force5Bytes = true"); //8 bits will do Write8(0x70 + conditionCode); Write8((u8)(s8)distance); @@ -407,7 +408,9 @@ void XEmitter::SetJumpTarget(const FixupBranch &branch) { if (branch.type == 0) { - branch.ptr[-1] = (u8)(s8)(code - branch.ptr); + s32 distance = (s32)(code - branch.ptr); + _assert_msg_(DYNA_REC, distance < 0x80, "Jump target too far away, needs force5Bytes = true"); + branch.ptr[-1] = (u8)(s8)distance; } else if (branch.type == 1) { diff --git a/Source/Core/DSPCore/Src/DSPEmitter.cpp b/Source/Core/DSPCore/Src/DSPEmitter.cpp index 3184e340d8..4b908847fe 100644 --- a/Source/Core/DSPCore/Src/DSPEmitter.cpp +++ b/Source/Core/DSPCore/Src/DSPEmitter.cpp @@ -194,6 +194,8 @@ const u8 *DSPEmitter::Compile(int start_addr) { SetJumpTarget(rLoopCounterExit); } + blockSize[start_addr]++; + // End the block if we're at a loop end. if (opcode->branch || (DSPAnalyzer::code_flags[addr] & DSPAnalyzer::CODE_LOOP_END) || @@ -201,8 +203,6 @@ const u8 *DSPEmitter::Compile(int start_addr) { break; } addr += opcode->size; - - blockSize[start_addr]++; } // ABI_RestoreStack(0); @@ -210,6 +210,13 @@ const u8 *DSPEmitter::Compile(int start_addr) { RET(); blocks[start_addr] = (CompiledCode)entryPoint; + if (blockSize[start_addr] == 0) + { + // just a safeguard, should never happen anymore. + // if it does we might get stuck over in RunForCycles. + ERROR_LOG(DSPLLE, "Block at 0x%04x has zero size", start_addr); + blockSize[start_addr] = 1; + } return entryPoint; } diff --git a/Source/Core/DSPCore/Src/DSPEmitter.h b/Source/Core/DSPCore/Src/DSPEmitter.h index e09e072bae..2a9cc80c46 100644 --- a/Source/Core/DSPCore/Src/DSPEmitter.h +++ b/Source/Core/DSPCore/Src/DSPEmitter.h @@ -117,6 +117,8 @@ private: DISALLOW_COPY_AND_ASSIGN(DSPEmitter); void ToMask(Gen::X64Reg value_reg = Gen::EDI, Gen::X64Reg temp_reg = Gen::ESI); + void dsp_increment_one(Gen::X64Reg ar = Gen::EAX, Gen::X64Reg wr = Gen::EDX, Gen::X64Reg wr_pow = Gen::EDI, Gen::X64Reg temp_reg = Gen::ESI); + void dsp_decrement_one(Gen::X64Reg ar = Gen::EAX, Gen::X64Reg wr = Gen::EDX, Gen::X64Reg wr_pow = Gen::EDI, Gen::X64Reg temp_reg = Gen::ESI); }; diff --git a/Source/Core/DSPCore/Src/Jit/DSPJitUtil.cpp b/Source/Core/DSPCore/Src/Jit/DSPJitUtil.cpp index b8ea950f1d..fd49d12672 100644 --- a/Source/Core/DSPCore/Src/Jit/DSPJitUtil.cpp +++ b/Source/Core/DSPCore/Src/Jit/DSPJitUtil.cpp @@ -45,6 +45,25 @@ void DSPEmitter::ToMask(X64Reg value_reg, X64Reg temp_reg) // HORRIBLE UGLINESS, someone please fix. // See http://code.google.com/p/dolphin-emu/source/detail?r=3125 +void DSPEmitter::dsp_increment_one(X64Reg ar, X64Reg wr, X64Reg wr_pow, X64Reg temp_reg) +{ + // if ((tmp & tmb) == tmb) + MOV(16, R(temp_reg), R(ar)); + AND(16, R(temp_reg), R(wr_pow)); + CMP(16, R(temp_reg), R(wr_pow)); + FixupBranch not_equal = J_CC(CC_NE); + + // tmp -= wr_reg + SUB(16, R(ar), R(wr)); + + FixupBranch end = J(); + SetJumpTarget(not_equal); + + // else tmp++ + ADD(16, R(ar), Imm16(1)); + SetJumpTarget(end); +} + // EAX = g_dsp.r[reg] // EDX = g_dsp.r[DSP_REG_WR0 + reg] void DSPEmitter::increment_addr_reg(int reg) @@ -57,27 +76,40 @@ void DSPEmitter::increment_addr_reg(int reg) MOV(16, R(EDI), R(EDX)); ToMask(EDI); - // if ((tmp & tmb) == tmb) - MOV(16, R(ESI), R(EAX)); - AND(16, R(ESI), R(EDI)); - CMP(16, R(ESI), R(EDI)); - FixupBranch not_equal = J_CC(CC_NE); - - // tmp -= wr_reg - SUB(16, R(EAX), R(EDX)); - - FixupBranch end = J(); - SetJumpTarget(not_equal); - - // else tmp++ - ADD(16, R(EAX), Imm16(1)); - SetJumpTarget(end); + dsp_increment_one(EAX, EDX, EDI); // g_dsp.r[reg] = tmp; MOV(16, M(&g_dsp.r[reg]), R(EAX)); } // See http://code.google.com/p/dolphin-emu/source/detail?r=3125 +void DSPEmitter::dsp_decrement_one(X64Reg ar, X64Reg wr, X64Reg wr_pow, X64Reg temp_reg) +{ + // compute min from wr_pow and ar + // min = (tmb+1-ar)&tmb; + LEA(16, temp_reg, MDisp(wr_pow, 1)); + SUB(16, R(temp_reg), R(ar)); + AND(16, R(temp_reg), R(wr_pow)); + + // wr < min + CMP(16, R(wr), R(temp_reg)); + FixupBranch wr_lt_min = J_CC(CC_B); + // !min + TEST(16, R(temp_reg), R(temp_reg)); + FixupBranch min_zero = J_CC(CC_Z); + + // ar--; + SUB(16, R(ar), Imm16(1)); + FixupBranch end = J(); + + // ar += wr; + SetJumpTarget(wr_lt_min); + SetJumpTarget(min_zero); + ADD(16, R(ar), R(wr)); + + SetJumpTarget(end); +} + // EAX = g_dsp.r[reg] // EDX = g_dsp.r[DSP_REG_WR0 + reg] void DSPEmitter::decrement_addr_reg(int reg) @@ -90,29 +122,7 @@ void DSPEmitter::decrement_addr_reg(int reg) MOV(16, R(EDI), R(EDX)); ToMask(EDI); - //compute min from EDI and EAX - // min = (tmb+1-ar)&tmb; - LEA(16, ESI, MDisp(EDI, 1)); - SUB(16, R(ESI), R(EAX)); - AND(16, R(ESI), R(EDI)); - - // wr < min - CMP(16, R(EDX), R(ESI)); - FixupBranch wr_lt_min = J_CC(CC_B); - // !min - TEST(16, R(ESI), R(ESI)); - FixupBranch min_zero = J_CC(CC_Z); - - // ar--; - SUB(16, R(EAX), Imm16(1)); - FixupBranch end = J(); - - // ar += wr; - SetJumpTarget(wr_lt_min); - SetJumpTarget(min_zero); - ADD(16, R(EAX), R(EDX)); - - SetJumpTarget(end); + dsp_decrement_one(EAX, EDX, EDI); // g_dsp.r[reg] = tmp; MOV(16, M(&g_dsp.r[reg]), R(EAX)); @@ -125,56 +135,50 @@ void DSPEmitter::decrement_addr_reg(int reg) // EDI = tomask(EDX) void DSPEmitter::increase_addr_reg(int reg) { - MOV(16, R(ECX), M(&g_dsp.r[DSP_REG_IX0 + reg])); - //IX0 == 0, bail out + MOVZX(32, 16, ECX, M(&g_dsp.r[DSP_REG_IX0 + reg])); + // IX0 == 0, bail out + TEST(16, R(ECX), R(ECX)); - FixupBranch end = J_CC(CC_Z); + // code too long for a 5-byte jump + // TODO: optimize a bit, maybe merge loops? + FixupBranch end = J_CC(CC_Z, true); MOV(16, R(EAX), M(&g_dsp.r[reg])); MOV(16, R(EDX), M(&g_dsp.r[DSP_REG_WR0 + reg])); - //IX0 > 0 - FixupBranch neg = J_CC(CC_L); - //ToMask(WR0), calculating it into EDI + // ToMask(WR0), calculating it into EDI MOV(16, R(EDI), R(EDX)); ToMask(EDI); - //dsp_increment - JumpTarget loop_pos = GetCodePtr(); - // if ((tmp & tmb) == tmb) - MOV(16, R(ESI), R(EAX)); - AND(16, R(ESI), R(EDI)); - CMP(16, R(ESI), R(EDI)); - FixupBranch pos_neq = J_CC(CC_NE); + // IX0 > 0 + // TODO: ToMask flushes flags set by TEST, + // needs another CMP here. + CMP(16, R(ECX), Imm16(0)); + FixupBranch neg = J_CC(CC_L); - // tmp ^= wr_reg - XOR(16, R(EAX), R(EDX)); - FixupBranch pos_eq = J(); - SetJumpTarget(pos_neq); - // else tmp++ - ADD(16, R(EAX), Imm16(1)); - SetJumpTarget(pos_eq); + JumpTarget loop_pos = GetCodePtr(); + + // dsp_increment + dsp_increment_one(EAX, EDX, EDI); SUB(16, R(ECX), Imm16(1)); // value-- + CMP(16, M(&g_dsp.r[DSP_REG_IX0 + reg]), Imm16(127)); + FixupBranch dbg = J_CC(CC_NE); + CMP(16, R(ECX), Imm16(1)); + FixupBranch dbg2 = J_CC(CC_NE); + INT3(); + SetJumpTarget(dbg2); + SetJumpTarget(dbg); CMP(16, R(ECX), Imm16(0)); // value > 0 J_CC(CC_G, loop_pos); FixupBranch end_pos = J(); - //else, IX0 < 0 + // else, IX0 < 0 SetJumpTarget(neg); JumpTarget loop_neg = GetCodePtr(); - //dsp_decrement - // if ((tmp & wr_reg) == 0) - TEST(16, R(EAX), R(EDX)); - FixupBranch neg_nz = J_CC(CC_NZ); - // tmp |= wr_reg; - OR(16, R(EAX), R(EDX)); - FixupBranch neg_z = J(); - SetJumpTarget(neg_nz); - // else tmp-- - SUB(16, R(EAX), Imm16(1)); - SetJumpTarget(neg_z); + // dsp_decrement + dsp_decrement_one(EAX, EDX, EDI); ADD(16, R(ECX), Imm16(1)); // value++ CMP(16, R(ECX), Imm16(0)); // value < 0 @@ -196,57 +200,41 @@ void DSPEmitter::increase_addr_reg(int reg) void DSPEmitter::decrease_addr_reg(int reg) { MOV(16, R(ECX), M(&g_dsp.r[DSP_REG_IX0 + reg])); - //IX0 == 0, bail out + // IX0 == 0, bail out TEST(16, R(ECX), R(ECX)); - FixupBranch end = J_CC(CC_Z); + // code too long for a 5-byte jump + // TODO: optimize a bit, maybe merge loops? + FixupBranch end = J_CC(CC_Z, true); MOV(16, R(EAX), M(&g_dsp.r[reg])); MOV(16, R(EDX), M(&g_dsp.r[DSP_REG_WR0 + reg])); - //IX0 > 0 + + // ToMask(WR0), calculating it into EDI + MOV(16, R(EDI), R(EDX)); + ToMask(EDI); + + // IX0 > 0 + // TODO: ToMask flushes flags set by TEST, + // needs another CMP here. + CMP(16, R(ECX), Imm16(0)); FixupBranch neg = J_CC(CC_L); + JumpTarget loop_pos = GetCodePtr(); - //dsp_decrement - // if ((tmp & wr_reg) == 0) - TEST(16, R(EAX), R(EDX)); - - FixupBranch neg_nz = J_CC(CC_NZ); - // tmp |= wr_reg; - OR(16, R(EAX), R(EDX)); - FixupBranch neg_z = J(); - SetJumpTarget(neg_nz); - // else tmp-- - SUB(16, R(EAX), Imm16(1)); - SetJumpTarget(neg_z); + // dsp_decrement + dsp_decrement_one(EAX, EDX, EDI); SUB(16, R(ECX), Imm16(1)); // value-- CMP(16, R(ECX), Imm16(0)); // value > 0 J_CC(CC_G, loop_pos); FixupBranch end_pos = J(); - //else, IX0 < 0 + // else, IX0 < 0 SetJumpTarget(neg); - - //ToMask(WR0), calculating it into EDI - MOV(16, R(EDI), R(EDX)); - ToMask(EDI); - JumpTarget loop_neg = GetCodePtr(); - //dsp_increment - // if ((tmp & tmb) == tmb) - MOV(16, R(ESI), R(EAX)); - AND(16, R(ESI), R(EDI)); - CMP(16, R(ESI), R(EDI)); - FixupBranch pos_neq = J_CC(CC_NE); - // tmp ^= wr_reg - XOR(16, R(EAX), R(EDX)); - FixupBranch pos_eq = J(); - - SetJumpTarget(pos_neq); - // else tmp++ - ADD(16, R(EAX), Imm16(1)); - SetJumpTarget(pos_eq); + // dsp_increment + dsp_increment_one(EAX, EDX, EDI); ADD(16, R(ECX), Imm16(1)); // value++ CMP(16, R(ECX), Imm16(0)); // value < 0