From 4114a0b50660e0821f4f54ddd597372a99c75a48 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 1 Sep 2024 16:10:17 +0200 Subject: [PATCH] Jit: Update constant propagation during instruction This commit makes the JIT set/clear the individual registers of ConstantPropagation immediately instead of at the end of the instruction. This is needed to prevent Jit64::ComputeRC, which reads from a register written to earlier during the same instruction, from reading back stale register values from ConstantPropagation in the next commit. --- Source/Core/Core/PowerPC/Jit64/Jit.cpp | 5 +++-- Source/Core/Core/PowerPC/Jit64/Jit.h | 2 ++ Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.cpp | 5 +++++ Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.h | 1 + Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.cpp | 6 ++++++ Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.h | 1 + Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp | 3 +++ Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.h | 1 + Source/Core/Core/PowerPC/JitArm64/Jit.cpp | 7 +++---- Source/Core/Core/PowerPC/JitArm64/Jit.h | 2 ++ Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp | 9 ++++++--- 11 files changed, 33 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.cpp b/Source/Core/Core/PowerPC/Jit64/Jit.cpp index 024b4c6e4af..bdbb023c258 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit.cpp @@ -370,6 +370,9 @@ void Jit64::FallBackToInterpreter(UGeckoInstruction inst) gpr.Reset(js.op->regsOut); fpr.Reset(js.op->GetFregsOut()); + // We must also update constant propagation + m_constant_propagation.ClearGPRs(js.op->regsOut); + if (js.op->opinfo->flags & FL_SET_MSR) EmitUpdateMembase(); @@ -1133,8 +1136,6 @@ bool Jit64::DoJit(u32 em_address, JitBlock* b, u32 nextPC) } CompileInstruction(op); - - m_constant_propagation.ClearGPRs(op.regsOut); } m_constant_propagation.Apply(constant_propagation_result); diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.h b/Source/Core/Core/PowerPC/Jit64/Jit.h index 189f0c2b4bb..8b96fda107a 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.h +++ b/Source/Core/Core/PowerPC/Jit64/Jit.h @@ -84,6 +84,8 @@ public: void FlushRegistersBeforeSlowAccess(); + JitCommon::ConstantPropagation& GetConstantPropagation() { return m_constant_propagation; } + JitBlockCache* GetBlockCache() override { return &blocks; } void Trace(); diff --git a/Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.cpp b/Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.cpp index df210f80c5b..64870ec026a 100644 --- a/Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.cpp +++ b/Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.cpp @@ -25,6 +25,11 @@ void FPURegCache::LoadRegister(preg_t preg, X64Reg new_loc) m_emitter->MOVAPD(new_loc, m_regs[preg].Location().value()); } +void FPURegCache::DiscardImm(preg_t preg) +{ + // FPURegCache doesn't support immediates, so no need to do anything +} + std::span FPURegCache::GetAllocationOrder() const { static constexpr X64Reg allocation_order[] = {XMM6, XMM7, XMM8, XMM9, XMM10, XMM11, XMM12, diff --git a/Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.h b/Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.h index f7d81663b6d..f34db9d0886 100644 --- a/Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.h +++ b/Source/Core/Core/PowerPC/Jit64/RegCache/FPURegCache.h @@ -16,6 +16,7 @@ protected: Gen::OpArg GetDefaultLocation(preg_t preg) const override; void StoreRegister(preg_t preg, const Gen::OpArg& newLoc) override; void LoadRegister(preg_t preg, Gen::X64Reg newLoc) override; + void DiscardImm(preg_t preg) override; std::span GetAllocationOrder() const override; BitSet32 GetRegUtilization() const override; BitSet32 CountRegsIn(preg_t preg, u32 lookahead) const override; diff --git a/Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.cpp b/Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.cpp index ca30e15784a..b44382ba447 100644 --- a/Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.cpp +++ b/Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.cpp @@ -25,6 +25,11 @@ void GPRRegCache::LoadRegister(preg_t preg, X64Reg new_loc) m_emitter->MOV(32, ::Gen::R(new_loc), m_regs[preg].Location().value()); } +void GPRRegCache::DiscardImm(preg_t preg) +{ + m_jit.GetConstantPropagation().ClearGPR(preg); +} + OpArg GPRRegCache::GetDefaultLocation(preg_t preg) const { return PPCSTATE_GPR(preg); @@ -50,6 +55,7 @@ void GPRRegCache::SetImmediate32(preg_t preg, u32 imm_value, bool dirty) // processing speculative constants. DiscardRegContentsIfCached(preg); m_regs[preg].SetToImm32(imm_value, dirty); + m_jit.GetConstantPropagation().SetGPR(preg, imm_value); } BitSet32 GPRRegCache::GetRegUtilization() const diff --git a/Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.h b/Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.h index 60985e19607..a5bf5242694 100644 --- a/Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.h +++ b/Source/Core/Core/PowerPC/Jit64/RegCache/GPRRegCache.h @@ -17,6 +17,7 @@ protected: Gen::OpArg GetDefaultLocation(preg_t preg) const override; void StoreRegister(preg_t preg, const Gen::OpArg& new_loc) override; void LoadRegister(preg_t preg, Gen::X64Reg new_loc) override; + void DiscardImm(preg_t preg) override; std::span GetAllocationOrder() const override; BitSet32 GetRegUtilization() const override; BitSet32 CountRegsIn(preg_t preg, u32 lookahead) const override; diff --git a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp index d59d4f0a01d..d1e06661709 100644 --- a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp +++ b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp @@ -536,6 +536,9 @@ void RegCache::BindToRegister(preg_t i, bool doLoad, bool makeDirty) m_xregs[RX(i)].MakeDirty(); } + if (makeDirty) + DiscardImm(i); + ASSERT_MSG(DYNA_REC, !m_xregs[RX(i)].IsLocked(), "WTF, this reg ({} -> {}) should have been flushed", i, Common::ToUnderlying(RX(i))); } diff --git a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.h b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.h index 3677d2b42b7..1ff4e27ea78 100644 --- a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.h +++ b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.h @@ -193,6 +193,7 @@ protected: virtual Gen::OpArg GetDefaultLocation(preg_t preg) const = 0; virtual void StoreRegister(preg_t preg, const Gen::OpArg& new_loc) = 0; virtual void LoadRegister(preg_t preg, Gen::X64Reg new_loc) = 0; + virtual void DiscardImm(preg_t preg) = 0; virtual std::span GetAllocationOrder() const = 0; diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp index ba30b0bc818..975f5f95f7d 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp @@ -279,6 +279,9 @@ void JitArm64::FallBackToInterpreter(UGeckoInstruction inst) fpr.ResetRegisters(js.op->GetFregsOut()); gpr.ResetCRRegisters(js.op->crOut); + // We must also update constant propagation + m_constant_propagation.ClearGPRs(js.op->regsOut); + if (js.op->opinfo->flags & FL_SET_MSR) EmitUpdateMembase(); @@ -1354,12 +1357,8 @@ bool JitArm64::DoJit(u32 em_address, JitBlock* b, u32 nextPC) m_constant_propagation.EvaluateInstruction(op.inst, opinfo->flags); if (!constant_propagation_result.instruction_fully_executed) - { CompileInstruction(op); - m_constant_propagation.ClearGPRs(op.regsOut); - } - m_constant_propagation.Apply(constant_propagation_result); if (constant_propagation_result.gpr >= 0) diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.h b/Source/Core/Core/PowerPC/JitArm64/Jit.h index 1b738061b22..e98f950d42f 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.h +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.h @@ -36,6 +36,8 @@ public: void Init() override; void Shutdown() override; + JitCommon::ConstantPropagation& GetConstantPropagation() { return m_constant_propagation; } + JitBaseBlockCache* GetBlockCache() override { return &blocks; } bool IsInCodeSpace(const u8* ptr) const { return IsInSpace(ptr); } bool HandleFault(uintptr_t access_address, SContext* ctx) override; diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp index 786f5be4a32..d0c9ac71bde 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp @@ -381,6 +381,7 @@ void Arm64GPRCache::SetImmediateInternal(size_t index, u32 imm, bool dirty) UnlockRegister(EncodeRegTo32(reg.GetReg())); reg.LoadToImm(imm); reg.SetDirty(dirty); + m_jit->GetConstantPropagation().SetGPR(index - GUEST_GPR_OFFSET, imm); } void Arm64GPRCache::BindForWrite(size_t index, bool will_read, bool will_write) @@ -388,6 +389,7 @@ void Arm64GPRCache::BindForWrite(size_t index, bool will_read, bool will_write) GuestRegInfo guest_reg = GetGuestByIndex(index); OpArg& reg = guest_reg.reg; const size_t bitsize = guest_reg.bitsize; + const bool is_gpr = index >= GUEST_GPR_OFFSET && index < GUEST_GPR_OFFSET + GUEST_GPR_COUNT; reg.ResetLastUsed(); @@ -414,12 +416,13 @@ void Arm64GPRCache::BindForWrite(size_t index, bool will_read, bool will_write) m_emit->MOVI2R(host_reg, reg.GetImm()); } reg.Load(host_reg); - if (will_write) - reg.SetDirty(true); } - else if (will_write) + + if (will_write) { reg.SetDirty(true); + if (is_gpr) + m_jit->GetConstantPropagation().ClearGPR(index - GUEST_GPR_OFFSET); } }