From e316d0e94feacf89a22bc1f841e76c8945326f4d Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 11 Jul 2021 00:00:03 +0200 Subject: [PATCH] JitArm64: Don't update dest reg when load triggers exception, part 2 If a host register has been newly allocated for the destination guest register, and the load triggers an exception, we must make sure to not write the old value in the host register into ppcState. This commit achieves this by not marking the register as dirty until after the load is done. --- .../PowerPC/JitArm64/JitArm64_LoadStore.cpp | 5 ++- .../PowerPC/JitArm64/JitArm64_RegCache.cpp | 41 +++++++++++-------- .../Core/PowerPC/JitArm64/JitArm64_RegCache.h | 14 +++++-- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_LoadStore.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_LoadStore.cpp index 38e693ca07..685d96c35a 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_LoadStore.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_LoadStore.cpp @@ -26,7 +26,7 @@ void JitArm64::SafeLoadToReg(u32 dest, s32 addr, s32 offsetReg, u32 flags, s32 o // We want to make sure to not get LR as a temp register gpr.Lock(ARM64Reg::W0, ARM64Reg::W30); - gpr.BindToRegister(dest, dest == (u32)addr || dest == (u32)offsetReg); + gpr.BindToRegister(dest, dest == (u32)addr || dest == (u32)offsetReg, false); ARM64Reg dest_reg = gpr.R(dest); ARM64Reg up_reg = ARM64Reg::INVALID_REG; ARM64Reg off_reg = ARM64Reg::INVALID_REG; @@ -135,6 +135,9 @@ void JitArm64::SafeLoadToReg(u32 dest, s32 addr, s32 offsetReg, u32 flags, s32 o EmitBackpatchRoutine(flags, jo.fastmem, jo.fastmem, dest_reg, XA, regs_in_use, fprs_in_use); } + gpr.BindToRegister(dest, false, true); + ASSERT(dest_reg == gpr.R(dest)); + gpr.Unlock(ARM64Reg::W0, ARM64Reg::W30); } diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp index 45249d92ed..24aae47c28 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp @@ -343,25 +343,29 @@ void Arm64GPRCache::SetImmediate(const GuestRegInfo& guest_reg, u32 imm) reg.LoadToImm(imm); } -void Arm64GPRCache::BindToRegister(const GuestRegInfo& guest_reg, bool do_load) +void Arm64GPRCache::BindToRegister(const GuestRegInfo& guest_reg, bool do_load, bool set_dirty) { OpArg& reg = guest_reg.reg; const size_t bitsize = guest_reg.bitsize; reg.ResetLastUsed(); - reg.SetDirty(true); const RegType reg_type = reg.GetType(); if (reg_type == RegType::NotLoaded || reg_type == RegType::Discarded) { const ARM64Reg host_reg = bitsize != 64 ? GetReg() : EncodeRegTo64(GetReg()); reg.Load(host_reg); + reg.SetDirty(set_dirty); if (do_load) { ASSERT_MSG(DYNA_REC, reg_type != RegType::Discarded, "Attempted to load a discarded value"); m_emit->LDR(IndexType::Unsigned, host_reg, PPC_REG, u32(guest_reg.ppc_offset)); } } + else if (set_dirty) + { + reg.SetDirty(true); + } } void Arm64GPRCache::GetAllocationOrder() @@ -570,26 +574,15 @@ ARM64Reg Arm64FPRCache::R(size_t preg, RegType type) return ARM64Reg::INVALID_REG; } -ARM64Reg Arm64FPRCache::RW(size_t preg, RegType type) +ARM64Reg Arm64FPRCache::RW(size_t preg, RegType type, bool set_dirty) { OpArg& reg = m_guest_registers[preg]; - bool was_dirty = reg.IsDirty(); - IncrementAllUsed(); reg.ResetLastUsed(); - reg.SetDirty(true); - - // If not loaded at all, just alloc a new one. - if (reg.GetType() == RegType::NotLoaded || reg.GetType() == RegType::Discarded) - { - reg.Load(GetReg(), type); - return reg.GetReg(); - } - // Only the lower value will be overwritten, so we must be extra careful to store PSR1 if dirty. - if ((type == RegType::LowerPair || type == RegType::LowerPairSingle) && was_dirty) + if (reg.IsDirty() && (type == RegType::LowerPair || type == RegType::LowerPairSingle)) { // We must *not* change host_reg as this register might still be in use. So it's fine to // store this register, but it's *not* fine to convert it to double. So for double conversion, @@ -612,6 +605,7 @@ ARM64Reg Arm64FPRCache::RW(size_t preg, RegType type) m_jit->ConvertSingleToDoubleLower(preg, flush_reg, flush_reg, scratch_reg); m_float_emit->STR(64, IndexType::Unsigned, flush_reg, PPC_REG, u32(PPCSTATE_OFF_PS1(preg))); Unlock(scratch_reg); + reg.Load(host_reg, RegType::LowerPairSingle); break; } else @@ -619,6 +613,7 @@ ARM64Reg Arm64FPRCache::RW(size_t preg, RegType type) m_jit->ConvertSingleToDoublePair(preg, flush_reg, host_reg, flush_reg); m_float_emit->STR(128, IndexType::Unsigned, flush_reg, PPC_REG, u32(PPCSTATE_OFF_PS0(preg))); + reg.SetDirty(false); } break; case RegType::Register: @@ -627,6 +622,7 @@ ARM64Reg Arm64FPRCache::RW(size_t preg, RegType type) // It would take longer to do an insert to a temporary and a 64bit store than to just do this. m_float_emit->STR(128, IndexType::Unsigned, flush_reg, PPC_REG, static_cast(PPCSTATE_OFF_PS0(preg))); + reg.SetDirty(false); break; case RegType::DuplicatedSingle: flush_reg = GetReg(); @@ -636,6 +632,8 @@ ARM64Reg Arm64FPRCache::RW(size_t preg, RegType type) // Store PSR1 (which is equal to PSR0) in memory. m_float_emit->STR(64, IndexType::Unsigned, flush_reg, PPC_REG, static_cast(PPCSTATE_OFF_PS1(preg))); + reg.Load(host_reg, reg.GetType() == RegType::DuplicatedSingle ? RegType::LowerPairSingle : + RegType::LowerPair); break; default: // All other types doesn't store anything in PSR1. @@ -646,7 +644,18 @@ ARM64Reg Arm64FPRCache::RW(size_t preg, RegType type) Unlock(flush_reg); } - reg.Load(reg.GetReg(), type); + if (reg.GetType() == RegType::NotLoaded || reg.GetType() == RegType::Discarded) + { + // If not loaded at all, just alloc a new one. + reg.Load(GetReg(), type); + reg.SetDirty(set_dirty); + } + else if (set_dirty) + { + reg.Load(reg.GetReg(), type); + reg.SetDirty(true); + } + return reg.GetReg(); } diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h index 2ecbcbbffa..16678a04b1 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h @@ -266,9 +266,15 @@ public: // Gets the immediate that a register is set to, only valid for guest GPRs u32 GetImm(size_t preg) const { return GetGuestGPROpArg(preg).GetImm(); } // Binds a guest GPR to a host register, optionally loading its value - void BindToRegister(size_t preg, bool do_load) { BindToRegister(GetGuestGPR(preg), do_load); } + void BindToRegister(size_t preg, bool do_load, bool set_dirty = true) + { + BindToRegister(GetGuestGPR(preg), do_load, set_dirty); + } // Binds a guest CR to a host register, optionally loading its value - void BindCRToRegister(size_t preg, bool do_load) { BindToRegister(GetGuestCR(preg), do_load); } + void BindCRToRegister(size_t preg, bool do_load, bool set_dirty = true) + { + BindToRegister(GetGuestCR(preg), do_load, set_dirty); + } BitSet32 GetCallerSavedUsed() const override; void StoreRegisters(BitSet32 regs, Arm64Gen::ARM64Reg tmp_reg = Arm64Gen::ARM64Reg::INVALID_REG) @@ -307,7 +313,7 @@ private: Arm64Gen::ARM64Reg R(const GuestRegInfo& guest_reg); void SetImmediate(const GuestRegInfo& guest_reg, u32 imm); - void BindToRegister(const GuestRegInfo& guest_reg, bool do_load); + void BindToRegister(const GuestRegInfo& guest_reg, bool do_load, bool set_dirty = true); void FlushRegisters(BitSet32 regs, bool maintain_state, Arm64Gen::ARM64Reg tmp_reg); void FlushCRRegisters(BitSet32 regs, bool maintain_state, Arm64Gen::ARM64Reg tmp_reg); @@ -326,7 +332,7 @@ public: // Will dump an immediate to the host register as well Arm64Gen::ARM64Reg R(size_t preg, RegType type); - Arm64Gen::ARM64Reg RW(size_t preg, RegType type); + Arm64Gen::ARM64Reg RW(size_t preg, RegType type, bool set_dirty = true); BitSet32 GetCallerSavedUsed() const override;