From e7ac6725224e9afb945b7abe048add2aec83c8ac Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 30 Dec 2017 00:31:46 -0800 Subject: [PATCH 1/5] arm64jit: Cleanup method names, temp discard. This way MapDirtyIn won't accidentally discard temps. --- Core/MIPS/ARM64/Arm64CompLoadStore.cpp | 10 +++++----- Core/MIPS/ARM64/Arm64RegCache.cpp | 26 ++++++++++++++++---------- Core/MIPS/ARM64/Arm64RegCache.h | 7 ++++--- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/Core/MIPS/ARM64/Arm64CompLoadStore.cpp b/Core/MIPS/ARM64/Arm64CompLoadStore.cpp index 856d3e71cb..a3f44e9e89 100644 --- a/Core/MIPS/ARM64/Arm64CompLoadStore.cpp +++ b/Core/MIPS/ARM64/Arm64CompLoadStore.cpp @@ -250,7 +250,7 @@ namespace MIPSComp { SetJumpTarget(skip); } - gpr.ReleaseSpillLocks(); + gpr.ReleaseSpillLocksAndDiscardTemps(); } void Arm64Jit::Comp_ITypeMem(MIPSOpcode op) { @@ -311,7 +311,7 @@ namespace MIPSComp { gpr.MapRegAsPointer(rs); // For a store, try to avoid mapping a reg if not needed. - targetReg = load ? INVALID_REG : gpr.MapTempImm(rt); + targetReg = load ? INVALID_REG : gpr.TryMapTempImm(rt); if (targetReg == INVALID_REG) { gpr.MapReg(rt, load ? MAP_NOINIT : 0); targetReg = gpr.R(rt); @@ -327,12 +327,12 @@ namespace MIPSComp { case 41: STRH(INDEX_UNSIGNED, targetReg, gpr.RPtr(rs), offset); break; case 40: STRB(INDEX_UNSIGNED, targetReg, gpr.RPtr(rs), offset); break; } - gpr.ReleaseSpillLocks(); + gpr.ReleaseSpillLocksAndDiscardTemps(); break; } } - if (!load && gpr.IsImm(rt) && gpr.MapTempImm(rt) != INVALID_REG) { + if (!load && gpr.IsImm(rt) && gpr.TryMapTempImm(rt) != INVALID_REG) { // We're storing an immediate value, let's see if we can optimize rt. if (!gpr.IsImm(rs) || offset == 0) { // In this case, we're always going to need rs mapped, which may flush the temp imm. @@ -340,7 +340,7 @@ namespace MIPSComp { gpr.MapIn(rs); } - targetReg = gpr.MapTempImm(rt); + targetReg = gpr.TryMapTempImm(rt); } if (gpr.IsImm(rs) && Memory::IsValidAddress(iaddr)) { diff --git a/Core/MIPS/ARM64/Arm64RegCache.cpp b/Core/MIPS/ARM64/Arm64RegCache.cpp index f9c8167784..0111ec00d2 100644 --- a/Core/MIPS/ARM64/Arm64RegCache.cpp +++ b/Core/MIPS/ARM64/Arm64RegCache.cpp @@ -304,7 +304,7 @@ ARM64Reg Arm64RegCache::FindBestToSpill(bool unusedOnly, bool *clobbered) { return INVALID_REG; } -ARM64Reg Arm64RegCache::MapTempImm(MIPSGPReg r) { +ARM64Reg Arm64RegCache::TryMapTempImm(MIPSGPReg r) { // If already mapped, no need for a temporary. if (IsMapped(r)) { return R(r); @@ -459,7 +459,7 @@ void Arm64RegCache::MapInIn(MIPSGPReg rd, MIPSGPReg rs) { SpillLock(rd, rs); MapReg(rd); MapReg(rs); - ReleaseSpillLocks(); + ReleaseSpillLock(rd, rs); } void Arm64RegCache::MapDirtyIn(MIPSGPReg rd, MIPSGPReg rs, bool avoidLoad) { @@ -467,7 +467,7 @@ void Arm64RegCache::MapDirtyIn(MIPSGPReg rd, MIPSGPReg rs, bool avoidLoad) { bool load = !avoidLoad || rd == rs; MapReg(rd, load ? MAP_DIRTY : MAP_NOINIT); MapReg(rs); - ReleaseSpillLocks(); + ReleaseSpillLock(rd, rs); } void Arm64RegCache::MapDirtyInIn(MIPSGPReg rd, MIPSGPReg rs, MIPSGPReg rt, bool avoidLoad) { @@ -476,7 +476,7 @@ void Arm64RegCache::MapDirtyInIn(MIPSGPReg rd, MIPSGPReg rs, MIPSGPReg rt, bool MapReg(rd, load ? MAP_DIRTY : MAP_NOINIT); MapReg(rt); MapReg(rs); - ReleaseSpillLocks(); + ReleaseSpillLock(rd, rs, rt); } void Arm64RegCache::MapDirtyDirtyIn(MIPSGPReg rd1, MIPSGPReg rd2, MIPSGPReg rs, bool avoidLoad) { @@ -486,7 +486,7 @@ void Arm64RegCache::MapDirtyDirtyIn(MIPSGPReg rd1, MIPSGPReg rd2, MIPSGPReg rs, MapReg(rd1, load1 ? MAP_DIRTY : MAP_NOINIT); MapReg(rd2, load2 ? MAP_DIRTY : MAP_NOINIT); MapReg(rs); - ReleaseSpillLocks(); + ReleaseSpillLock(rd1, rd2, rs); } void Arm64RegCache::MapDirtyDirtyInIn(MIPSGPReg rd1, MIPSGPReg rd2, MIPSGPReg rs, MIPSGPReg rt, bool avoidLoad) { @@ -497,7 +497,7 @@ void Arm64RegCache::MapDirtyDirtyInIn(MIPSGPReg rd1, MIPSGPReg rd2, MIPSGPReg rs MapReg(rd2, load2 ? MAP_DIRTY : MAP_NOINIT); MapReg(rt); MapReg(rs); - ReleaseSpillLocks(); + ReleaseSpillLock(rd1, rd2, rs, rt); } void Arm64RegCache::FlushArmReg(ARM64Reg r) { @@ -872,7 +872,7 @@ void Arm64RegCache::SpillLock(MIPSGPReg r1, MIPSGPReg r2, MIPSGPReg r3, MIPSGPRe if (r4 != MIPS_REG_INVALID) mr[r4].spillLock = true; } -void Arm64RegCache::ReleaseSpillLocks() { +void Arm64RegCache::ReleaseSpillLocksAndDiscardTemps() { for (int i = 0; i < NUM_MIPSREG; i++) { if (!mr[i].isStatic) mr[i].spillLock = false; @@ -882,9 +882,15 @@ void Arm64RegCache::ReleaseSpillLocks() { } } -void Arm64RegCache::ReleaseSpillLock(MIPSGPReg reg) { - if (!mr[reg].isStatic) - mr[reg].spillLock = false; +void Arm64RegCache::ReleaseSpillLock(MIPSGPReg r1, MIPSGPReg r2, MIPSGPReg r3, MIPSGPReg r4) { + if (!mr[r1].isStatic) + mr[r1].spillLock = false; + if (r2 != MIPS_REG_INVALID && !mr[r2].isStatic) + mr[r2].spillLock = false; + if (r3 != MIPS_REG_INVALID && !mr[r3].isStatic) + mr[r3].spillLock = false; + if (r4 != MIPS_REG_INVALID && !mr[r4].isStatic) + mr[r4].spillLock = false; } ARM64Reg Arm64RegCache::R(MIPSGPReg mipsReg) { diff --git a/Core/MIPS/ARM64/Arm64RegCache.h b/Core/MIPS/ARM64/Arm64RegCache.h index b81f4bbb0c..603dca1289 100644 --- a/Core/MIPS/ARM64/Arm64RegCache.h +++ b/Core/MIPS/ARM64/Arm64RegCache.h @@ -92,8 +92,8 @@ public: // Protect the arm register containing a MIPS register from spilling, to ensure that // it's being kept allocated. void SpillLock(MIPSGPReg reg, MIPSGPReg reg2 = MIPS_REG_INVALID, MIPSGPReg reg3 = MIPS_REG_INVALID, MIPSGPReg reg4 = MIPS_REG_INVALID); - void ReleaseSpillLock(MIPSGPReg reg); - void ReleaseSpillLocks(); + void ReleaseSpillLock(MIPSGPReg reg, MIPSGPReg reg2 = MIPS_REG_INVALID, MIPSGPReg reg3 = MIPS_REG_INVALID, MIPSGPReg reg4 = MIPS_REG_INVALID); + void ReleaseSpillLocksAndDiscardTemps(); void SetImm(MIPSGPReg reg, u64 immVal); bool IsImm(MIPSGPReg reg) const; @@ -102,7 +102,8 @@ public: // Optimally set a register to an imm value (possibly using another register.) void SetRegImm(Arm64Gen::ARM64Reg reg, u64 imm); - Arm64Gen::ARM64Reg MapTempImm(MIPSGPReg); + // May fail and return INVALID_REG if it needs flushing. + Arm64Gen::ARM64Reg TryMapTempImm(MIPSGPReg); // Returns an ARM register containing the requested MIPS register. Arm64Gen::ARM64Reg MapReg(MIPSGPReg reg, int mapFlags = 0); From 89cbf36611daa0bebb5d2d75b2fba5b7a3139b6b Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 30 Dec 2017 00:32:31 -0800 Subject: [PATCH 2/5] arm64jit: Free up W23 for static alloc. We shouldn't always reserve W23 for this uncommon case. --- Core/MIPS/ARM64/Arm64CompBranch.cpp | 6 ++++-- Core/MIPS/ARM64/Arm64RegCache.h | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Core/MIPS/ARM64/Arm64CompBranch.cpp b/Core/MIPS/ARM64/Arm64CompBranch.cpp index ea9acffa6f..9d8f41fb8c 100644 --- a/Core/MIPS/ARM64/Arm64CompBranch.cpp +++ b/Core/MIPS/ARM64/Arm64CompBranch.cpp @@ -536,7 +536,7 @@ void Arm64Jit::Comp_JumpReg(MIPSOpcode op) delaySlotIsNice = false; CONDITIONAL_NICE_DELAYSLOT; - ARM64Reg destReg = OTHERTEMPREG; + ARM64Reg destReg = INVALID_REG; if (IsSyscall(delaySlotOp)) { gpr.MapReg(rs); MovToPC(gpr.R(rs)); // For syscall to be able to return. @@ -574,7 +574,9 @@ void Arm64Jit::Comp_JumpReg(MIPSOpcode op) destReg = gpr.R(rs); // Safe because FlushAll doesn't change any regs FlushAll(); } else { - // Delay slot - this case is very rare, might be able to free up W24. + // Since we can't be in a delay slot, should be safe to steal FLAGTEMPREG for a temp reg. + // It will be saved, even if a function is called. + destReg = FLAGTEMPREG; gpr.MapReg(rs); MOV(destReg, gpr.R(rs)); if (andLink) diff --git a/Core/MIPS/ARM64/Arm64RegCache.h b/Core/MIPS/ARM64/Arm64RegCache.h index 603dca1289..b95caf7854 100644 --- a/Core/MIPS/ARM64/Arm64RegCache.h +++ b/Core/MIPS/ARM64/Arm64RegCache.h @@ -23,8 +23,7 @@ namespace Arm64JitConstants { -const Arm64Gen::ARM64Reg DOWNCOUNTREG = Arm64Gen::W23; -const Arm64Gen::ARM64Reg OTHERTEMPREG = Arm64Gen::W24; +const Arm64Gen::ARM64Reg DOWNCOUNTREG = Arm64Gen::W24; const Arm64Gen::ARM64Reg FLAGTEMPREG = Arm64Gen::X25; const Arm64Gen::ARM64Reg JITBASEREG = Arm64Gen::X26; const Arm64Gen::ARM64Reg CTXREG = Arm64Gen::X27; From ae636283602cecd02dd125d9faf897a667b40b8f Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 30 Dec 2017 00:34:36 -0800 Subject: [PATCH 3/5] arm64jit: Statically allocate ra as well. This doesn't seem to have a significant impact on performance, but it improves bloat by about 5%. --- Core/MIPS/ARM64/Arm64RegCache.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Core/MIPS/ARM64/Arm64RegCache.cpp b/Core/MIPS/ARM64/Arm64RegCache.cpp index 0111ec00d2..dd2c83c302 100644 --- a/Core/MIPS/ARM64/Arm64RegCache.cpp +++ b/Core/MIPS/ARM64/Arm64RegCache.cpp @@ -69,10 +69,10 @@ void Arm64RegCache::Start(MIPSAnalyst::AnalysisResults &stats) { const ARM64Reg *Arm64RegCache::GetMIPSAllocationOrder(int &count) { // See register alloc remarks in Arm64Asm.cpp - // W19-W22 are most suitable for static allocation. Those that are chosen for static allocation + // W19-W23 are most suitable for static allocation. Those that are chosen for static allocation // should be omitted here and added in GetStaticAllocations. static const ARM64Reg allocationOrder[] = { - W19, W20, W21, W22, W0, W1, W2, W3, W4, W5, W6, W7, W8, W9, W10, W11, W12, W13, W14, W15, + W19, W20, W21, W22, W23, W0, W1, W2, W3, W4, W5, W6, W7, W8, W9, W10, W11, W12, W13, W14, W15, }; static const ARM64Reg allocationOrderStaticAlloc[] = { W0, W1, W2, W3, W4, W5, W6, W7, W8, W9, W10, W11, W12, W13, W14, W15, @@ -93,6 +93,7 @@ const Arm64RegCache::StaticAllocation *Arm64RegCache::GetStaticAllocations(int & {MIPS_REG_V0, W20}, {MIPS_REG_V1, W22}, {MIPS_REG_A0, W21}, + {MIPS_REG_RA, W23}, }; if (jo_->useStaticAlloc) { From 79a9018aace7501c9392b0b3f2ffdee04081f94a Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 30 Dec 2017 00:37:34 -0800 Subject: [PATCH 4/5] Debugger: Fix hang when viewing invalid RAM. If you end up in disassembly near invalid RAM, bad things were happening, since hitting bad RAM updates the debugger, which hits bad RAM, which... --- Windows/Debugger/CtrlDisAsmView.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Windows/Debugger/CtrlDisAsmView.cpp b/Windows/Debugger/CtrlDisAsmView.cpp index 129d4a731a..fecc7de397 100644 --- a/Windows/Debugger/CtrlDisAsmView.cpp +++ b/Windows/Debugger/CtrlDisAsmView.cpp @@ -228,7 +228,8 @@ bool CtrlDisAsmView::getDisasmAddressText(u32 address, char* dest, bool abbrevia } } else { if (showData) { - sprintf(dest, "%08X %08X", address, Memory::Read_Instruction(address, true).encoding); + u32 encoding = Memory::IsValidAddress(address) ? Memory::Read_Instruction(address, true).encoding : 0; + sprintf(dest, "%08X %08X", address, encoding); } else { sprintf(dest, "%08X", address); } From 9ff812b31352724cdca6135da7ab2077dfa6ff74 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 30 Dec 2017 10:57:50 -0800 Subject: [PATCH 5/5] arm64jit: Negate in ADDI2R/SUBI2R as well. Should've done this at the same time as CMN. It's not as common, mostly catches addu calls, but it's good to have these generic for other uses. --- Common/Arm64Emitter.cpp | 8 ++++++++ Core/MIPS/ARM64/Arm64CompALU.cpp | 6 +----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Common/Arm64Emitter.cpp b/Common/Arm64Emitter.cpp index 52bf53e1f7..abf9747601 100644 --- a/Common/Arm64Emitter.cpp +++ b/Common/Arm64Emitter.cpp @@ -3736,6 +3736,7 @@ void ARM64XEmitter::CMPI2R(ARM64Reg Rn, u64 imm, ARM64Reg scratch) { } bool ARM64XEmitter::TryADDI2R(ARM64Reg Rd, ARM64Reg Rn, u64 imm) { + s64 negated = Is64Bit(Rn) ? -(s64)imm : -(s32)(u32)imm; u32 val; bool shift; if (imm == 0) { @@ -3745,12 +3746,16 @@ bool ARM64XEmitter::TryADDI2R(ARM64Reg Rd, ARM64Reg Rn, u64 imm) { } else if (IsImmArithmetic(imm, &val, &shift)) { ADD(Rd, Rn, val, shift); return true; + } else if (IsImmArithmetic((u64)negated, &val, &shift)) { + SUB(Rd, Rn, val, shift); + return true; } else { return false; } } bool ARM64XEmitter::TrySUBI2R(ARM64Reg Rd, ARM64Reg Rn, u64 imm) { + s64 negated = Is64Bit(Rn) ? -(s64)imm : -(s32)(u32)imm; u32 val; bool shift; if (imm == 0) { @@ -3760,6 +3765,9 @@ bool ARM64XEmitter::TrySUBI2R(ARM64Reg Rd, ARM64Reg Rn, u64 imm) { } else if (IsImmArithmetic(imm, &val, &shift)) { SUB(Rd, Rn, val, shift); return true; + } else if (IsImmArithmetic((u64)negated, &val, &shift)) { + ADD(Rd, Rn, val, shift); + return true; } else { return false; } diff --git a/Core/MIPS/ARM64/Arm64CompALU.cpp b/Core/MIPS/ARM64/Arm64CompALU.cpp index e9400f1efa..48572f3d96 100644 --- a/Core/MIPS/ARM64/Arm64CompALU.cpp +++ b/Core/MIPS/ARM64/Arm64CompALU.cpp @@ -90,11 +90,7 @@ void Arm64Jit::Comp_IType(MIPSOpcode op) { ARM64Reg r32 = gpr.RPtr(rs); gpr.MarkDirty(r32); ARM64Reg r = EncodeRegTo64(r32); - if (simm > 0) { - ADDI2R(r, r, simm); - } else { - SUBI2R(r, r, -simm); - } + ADDI2R(r, r, simm); } else { if (simm >= 0) { CompImmLogic(rs, rt, simm, &ARM64XEmitter::ADD, &ARM64XEmitter::TryADDI2R, &EvalAdd);