From 0c024de5911ad29e8a88663a45c1447de6eadaa3 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 29 Nov 2025 11:40:05 +0100 Subject: [PATCH] Jit64: Flush carry flag in FallBackToInterpreter We have an optimization where the guest carry flag is kept in the host carry flag between certain back-to-back pairs of integer instructions. If the second instruction falls back to the interpreter, then FallBackToInterpreter should flush the carry flag to m_ppc_state, otherwise the interpreter reads a stale carry flag and at some later point Jit64 trips the "Attempt to modify flags while flags locked!" assertion. An alternative solution would be to not store the guest carry flag in the host carry flag to begin with if we know the next instruction is going to fall back to the interpreter, but knowing that in advance is non-trivial. Since interpreter fallbacks aren't exactly intended to be super optimized, I went for the flushing solution instead, which is how JitArm64 already works. In most cases, the emitted code shouldn't even differ between these two solutions. Note that the problematic situation only happens if the first integer instruction doesn't fall back to the interpreter but the second one does. This used to be impossible because there's no "JIT disable" setting that's granular enough to disable some integer instructions but not all, but with the constant propagation PR, it's possible if constant propagation is able to entirely evaluate the first instruction but not the second. --- Source/Core/Core/PowerPC/Jit64/Jit.cpp | 1 + Source/Core/Core/PowerPC/Jit64/Jit.h | 1 + .../Core/Core/PowerPC/Jit64/Jit_Integer.cpp | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+) diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.cpp b/Source/Core/Core/PowerPC/Jit64/Jit.cpp index 6cee5b971e3..1cdaf8c55ac 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit.cpp @@ -351,6 +351,7 @@ void Jit64::Shutdown() void Jit64::FallBackToInterpreter(UGeckoInstruction inst) { + FlushCarry(); gpr.Flush(BitSet32(0xFFFFFFFF), RegCache::IgnoreDiscardedRegisters::Yes); fpr.Flush(BitSet32(0xFFFFFFFF), RegCache::IgnoreDiscardedRegisters::Yes); diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.h b/Source/Core/Core/PowerPC/Jit64/Jit.h index 8b96fda107a..ad5db1fa103 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.h +++ b/Source/Core/Core/PowerPC/Jit64/Jit.h @@ -124,6 +124,7 @@ public: void FinalizeCarryOverflow(bool oe, bool inv = false); void FinalizeCarry(Gen::CCFlags cond); void FinalizeCarry(bool ca); + void FlushCarry(); void ComputeRC(preg_t preg, bool needs_test = true, bool needs_sext = true); void FinalizeImmediateRC(s32 value); diff --git a/Source/Core/Core/PowerPC/Jit64/Jit_Integer.cpp b/Source/Core/Core/PowerPC/Jit64/Jit_Integer.cpp index 03bf2fc7867..8ce73201f08 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit_Integer.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit_Integer.cpp @@ -137,6 +137,25 @@ void Jit64::FinalizeCarryOverflow(bool oe, bool inv) FinalizeCarry(inv ? CC_NC : CC_C); } +void Jit64::FlushCarry() +{ + switch (js.carryFlag) + { + case CarryFlag::InPPCState: + break; + case CarryFlag::InHostCarry: + JitSetCAIf(CC_C); + UnlockFlags(); + break; + case CarryFlag::InHostCarryInverted: + JitSetCAIf(CC_NC); + UnlockFlags(); + break; + } + + js.carryFlag = CarryFlag::InPPCState; +} + // Be careful; only set needs_test to false if we can be absolutely sure flags don't need // to be recalculated and haven't been clobbered. Keep in mind not all instructions set // sufficient flags -- for example, the flags from SHL/SHR are *not* sufficient for LT/GT