From f58abf59c0e45ccee7844f651b70f1a4af106f97 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 1 Apr 2024 16:04:23 +0200 Subject: [PATCH] Jit: Skip discarded registers when flushing for interpreter fallback Normally, the asserts added in 34b0a6ea90 are only triggered when something actually went wrong in Dolphin. But there is one exception: In FallBackToInterpreter, we flush all registers regardless of whether they're discarded. This is fine as long as none of the discarded registers are inputs to the instruction that the interpreter will run. To avoid false positive asserts, this change adds a parameter to Flush that controls whether to skip the asserts for discarded registers. Additionally, an assert for discarded registers is added to Arm64FPRCache::Flush. (Previously JitArm64 asserted for GPRs (and CRs) only, whereas Jit64 asserted both for GPRs and FPRs. I most likely didn't think of FPRs when writing 34b0a6ea90.) --- Source/Core/Core/PowerPC/Jit64/Jit.cpp | 4 +-- .../PowerPC/Jit64/RegCache/JitRegCache.cpp | 5 +-- .../Core/PowerPC/Jit64/RegCache/JitRegCache.h | 9 ++++- Source/Core/Core/PowerPC/JitArm64/Jit.cpp | 4 +-- .../PowerPC/JitArm64/JitArm64_RegCache.cpp | 33 +++++++++++++------ .../Core/PowerPC/JitArm64/JitArm64_RegCache.h | 27 +++++++++++---- 6 files changed, 58 insertions(+), 24 deletions(-) diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.cpp b/Source/Core/Core/PowerPC/Jit64/Jit.cpp index ce1dc906d3..3c4ab352c0 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit.cpp @@ -334,8 +334,8 @@ void Jit64::Shutdown() void Jit64::FallBackToInterpreter(UGeckoInstruction inst) { - gpr.Flush(); - fpr.Flush(); + gpr.Flush(BitSet32(0xFFFFFFFF), RegCache::IgnoreDiscardedRegisters::Yes); + fpr.Flush(BitSet32(0xFFFFFFFF), RegCache::IgnoreDiscardedRegisters::Yes); if (js.op->canEndBlock) { diff --git a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp index 86f1d9e15a..f80fd0943e 100644 --- a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp +++ b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp @@ -404,7 +404,7 @@ void RegCache::Discard(BitSet32 pregs) } } -void RegCache::Flush(BitSet32 pregs) +void RegCache::Flush(BitSet32 pregs, IgnoreDiscardedRegisters ignore_discarded_registers) { ASSERT_MSG( DYNA_REC, @@ -423,7 +423,8 @@ void RegCache::Flush(BitSet32 pregs) case PPCCachedReg::LocationType::Default: break; case PPCCachedReg::LocationType::Discarded: - ASSERT_MSG(DYNA_REC, false, "Attempted to flush discarded PPC reg {}", i); + ASSERT_MSG(DYNA_REC, ignore_discarded_registers != IgnoreDiscardedRegisters::No, + "Attempted to flush discarded PPC reg {}", i); break; case PPCCachedReg::LocationType::SpeculativeImmediate: // We can have a cached value without a host register through speculative constants. diff --git a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.h b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.h index 158c41c02d..d926216487 100644 --- a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.h +++ b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.h @@ -129,6 +129,12 @@ public: MaintainState, }; + enum class IgnoreDiscardedRegisters + { + No, + Yes, + }; + explicit RegCache(Jit64& jit); virtual ~RegCache() = default; @@ -169,7 +175,8 @@ public: RCForkGuard Fork(); void Discard(BitSet32 pregs); - void Flush(BitSet32 pregs = BitSet32::AllTrue(32)); + void Flush(BitSet32 pregs = BitSet32::AllTrue(32), + IgnoreDiscardedRegisters ignore_discarded_registers = IgnoreDiscardedRegisters::No); void Reset(BitSet32 pregs); void Revert(); void Commit(); diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp index b8fada1dcb..3fdc819561 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp @@ -184,8 +184,8 @@ void JitArm64::Shutdown() void JitArm64::FallBackToInterpreter(UGeckoInstruction inst) { FlushCarry(); - gpr.Flush(FlushMode::All, ARM64Reg::INVALID_REG); - fpr.Flush(FlushMode::All, ARM64Reg::INVALID_REG); + gpr.Flush(FlushMode::All, ARM64Reg::INVALID_REG, IgnoreDiscardedRegisters::Yes); + fpr.Flush(FlushMode::All, ARM64Reg::INVALID_REG, IgnoreDiscardedRegisters::Yes); if (js.op->canEndBlock) { diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp index a82949bf6e..ddede86dfd 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp @@ -241,12 +241,16 @@ void Arm64GPRCache::FlushRegister(size_t index, FlushMode mode, ARM64Reg tmp_reg } } -void Arm64GPRCache::FlushRegisters(BitSet32 regs, FlushMode mode, ARM64Reg tmp_reg) +void Arm64GPRCache::FlushRegisters(BitSet32 regs, FlushMode mode, ARM64Reg tmp_reg, + IgnoreDiscardedRegisters ignore_discarded_registers) { for (auto iter = regs.begin(); iter != regs.end(); ++iter) { const int i = *iter; - ASSERT_MSG(DYNA_REC, m_guest_registers[GUEST_GPR_OFFSET + i].GetType() != RegType::Discarded, + + ASSERT_MSG(DYNA_REC, + ignore_discarded_registers != IgnoreDiscardedRegisters::No || + m_guest_registers[GUEST_GPR_OFFSET + i].GetType() != RegType::Discarded, "Attempted to flush discarded register"); if (i + 1 < int(GUEST_GPR_COUNT) && regs[i + 1]) @@ -280,11 +284,14 @@ void Arm64GPRCache::FlushRegisters(BitSet32 regs, FlushMode mode, ARM64Reg tmp_r } } -void Arm64GPRCache::FlushCRRegisters(BitSet8 regs, FlushMode mode, ARM64Reg tmp_reg) +void Arm64GPRCache::FlushCRRegisters(BitSet8 regs, FlushMode mode, ARM64Reg tmp_reg, + IgnoreDiscardedRegisters ignore_discarded_registers) { for (int i : regs) { - ASSERT_MSG(DYNA_REC, m_guest_registers[GUEST_CR_OFFSET + i].GetType() != RegType::Discarded, + ASSERT_MSG(DYNA_REC, + ignore_discarded_registers != IgnoreDiscardedRegisters::No || + m_guest_registers[GUEST_CR_OFFSET + i].GetType() != RegType::Discarded, "Attempted to flush discarded register"); FlushRegister(GUEST_CR_OFFSET + i, mode, tmp_reg); @@ -310,10 +317,11 @@ void Arm64GPRCache::ResetCRRegisters(BitSet8 regs) } } -void Arm64GPRCache::Flush(FlushMode mode, ARM64Reg tmp_reg) +void Arm64GPRCache::Flush(FlushMode mode, ARM64Reg tmp_reg, + IgnoreDiscardedRegisters ignore_discarded_registers) { - FlushRegisters(BitSet32(0xFFFFFFFF), mode, tmp_reg); - FlushCRRegisters(BitSet8(0xFF), mode, tmp_reg); + FlushRegisters(BitSet32(0xFFFFFFFF), mode, tmp_reg, ignore_discarded_registers); + FlushCRRegisters(BitSet8(0xFF), mode, tmp_reg, ignore_discarded_registers); } ARM64Reg Arm64GPRCache::R(const GuestRegInfo& guest_reg) @@ -490,14 +498,19 @@ Arm64FPRCache::Arm64FPRCache() : Arm64RegCache(GUEST_FPR_COUNT) { } -void Arm64FPRCache::Flush(FlushMode mode, ARM64Reg tmp_reg) +void Arm64FPRCache::Flush(FlushMode mode, ARM64Reg tmp_reg, + IgnoreDiscardedRegisters ignore_discarded_registers) { for (size_t i = 0; i < m_guest_registers.size(); ++i) { const RegType reg_type = m_guest_registers[i].GetType(); - if (reg_type != RegType::NotLoaded && reg_type != RegType::Discarded && - reg_type != RegType::Immediate) + if (reg_type == RegType::Discarded) + { + ASSERT_MSG(DYNA_REC, ignore_discarded_registers != IgnoreDiscardedRegisters::No, + "Attempted to flush discarded register"); + } + else if (reg_type != RegType::NotLoaded && reg_type != RegType::Immediate) { FlushRegister(i, mode, tmp_reg); } diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h index 9ea4e0e3b6..8895375958 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h @@ -81,6 +81,12 @@ enum class FlushMode : bool MaintainState, }; +enum class IgnoreDiscardedRegisters +{ + No, + Yes, +}; + class OpArg { public: @@ -172,7 +178,8 @@ public: // Flushes the register cache in different ways depending on the mode. // A temporary register must be supplied when flushing GPRs with FlushMode::MaintainState, // but in other cases it can be set to ARM64Reg::INVALID_REG when convenient for the caller. - virtual void Flush(FlushMode mode, Arm64Gen::ARM64Reg tmp_reg) = 0; + virtual void Flush(FlushMode mode, Arm64Gen::ARM64Reg tmp_reg, + IgnoreDiscardedRegisters ignore_discarded_registers) = 0; virtual BitSet32 GetCallerSavedUsed() const = 0; @@ -265,7 +272,9 @@ public: // Flushes the register cache in different ways depending on the mode. // A temporary register must be supplied when flushing GPRs with FlushMode::MaintainState, // but in other cases it can be set to ARM64Reg::INVALID_REG when convenient for the caller. - void Flush(FlushMode mode, Arm64Gen::ARM64Reg tmp_reg) override; + void Flush( + FlushMode mode, Arm64Gen::ARM64Reg tmp_reg, + IgnoreDiscardedRegisters ignore_discarded_registers = IgnoreDiscardedRegisters::No) override; // Returns a guest GPR inside of a host register. // Will dump an immediate to the host register as well. @@ -329,12 +338,12 @@ public: void StoreRegisters(BitSet32 regs, Arm64Gen::ARM64Reg tmp_reg = Arm64Gen::ARM64Reg::INVALID_REG) { - FlushRegisters(regs, FlushMode::All, tmp_reg); + FlushRegisters(regs, FlushMode::All, tmp_reg, IgnoreDiscardedRegisters::No); } void StoreCRRegisters(BitSet8 regs, Arm64Gen::ARM64Reg tmp_reg = Arm64Gen::ARM64Reg::INVALID_REG) { - FlushCRRegisters(regs, FlushMode::All, tmp_reg); + FlushCRRegisters(regs, FlushMode::All, tmp_reg, IgnoreDiscardedRegisters::No); } void DiscardCRRegisters(BitSet8 regs); @@ -369,8 +378,10 @@ private: void SetImmediate(const GuestRegInfo& guest_reg, u32 imm, bool dirty); void BindToRegister(const GuestRegInfo& guest_reg, bool will_read, bool will_write = true); - void FlushRegisters(BitSet32 regs, FlushMode mode, Arm64Gen::ARM64Reg tmp_reg); - void FlushCRRegisters(BitSet8 regs, FlushMode mode, Arm64Gen::ARM64Reg tmp_reg); + void FlushRegisters(BitSet32 regs, FlushMode mode, Arm64Gen::ARM64Reg tmp_reg, + IgnoreDiscardedRegisters ignore_discarded_registers); + void FlushCRRegisters(BitSet8 regs, FlushMode mode, Arm64Gen::ARM64Reg tmp_reg, + IgnoreDiscardedRegisters ignore_discarded_registers); }; class Arm64FPRCache : public Arm64RegCache @@ -380,7 +391,9 @@ public: // Flushes the register cache in different ways depending on the mode. // The temporary register can be set to ARM64Reg::INVALID_REG when convenient for the caller. - void Flush(FlushMode mode, Arm64Gen::ARM64Reg tmp_reg) override; + void Flush( + FlushMode mode, Arm64Gen::ARM64Reg tmp_reg, + IgnoreDiscardedRegisters ignore_discarded_registers = IgnoreDiscardedRegisters::No) override; // Returns a guest register inside of a host register // Will dump an immediate to the host register as well