From 5902b5b11351625d347a694f5aadde5d7ade4aed Mon Sep 17 00:00:00 2001 From: JosJuice Date: Fri, 25 Aug 2023 16:15:54 +0200 Subject: [PATCH 1/2] PPCAnalyst: Don't discard before gather pipe interrupt check This bug has been lurking in the code ever since I added the discard functionality. It doesn't seem to be triggered all that often, and on top of that the emitted code only runs conditionally, so I'm not sure if people have been affected by this bug in practice or not. --- Source/Core/Core/PowerPC/PPCAnalyst.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Source/Core/Core/PowerPC/PPCAnalyst.cpp b/Source/Core/Core/PowerPC/PPCAnalyst.cpp index 8e6a1eee00..78a80e80a2 100644 --- a/Source/Core/Core/PowerPC/PPCAnalyst.cpp +++ b/Source/Core/Core/PowerPC/PPCAnalyst.cpp @@ -757,6 +757,16 @@ bool PPCAnalyzer::IsBusyWaitLoop(CodeBlock* block, CodeOp* code, size_t instruct return false; } +static bool CanCauseGatherPipeInterruptCheck(const CodeOp& op) +{ + // eieio + if (op.inst.OPCD == 31 && op.inst.SUBOP10 == 854) + return true; + + return op.opinfo->type == OpType::Store || op.opinfo->type == OpType::StoreFP || + op.opinfo->type == OpType::StorePS; +} + u32 PPCAnalyzer::Analyze(u32 address, CodeBlock* block, CodeBuffer* buffer, std::size_t block_size) const { @@ -952,6 +962,14 @@ u32 PPCAnalyzer::Analyze(u32 address, CodeBlock* block, CodeBuffer* buffer, { CodeOp& op = code[i]; + if (CanCauseGatherPipeInterruptCheck(op)) + { + // If we're doing a gather pipe interrupt check after this instruction, we need to + // be able to flush all registers, so we can't have any discarded registers. + gprDiscardable = BitSet32{}; + fprDiscardable = BitSet32{}; + } + const BitSet8 opWantsCR = op.wantsCR; const bool opWantsFPRF = op.wantsFPRF; const bool opWantsCA = op.wantsCA; From 34b0a6ea90b2a829ced39639d60fbdfbb7af4632 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Fri, 25 Aug 2023 16:06:39 +0200 Subject: [PATCH 2/2] Jit: Check for discarded registers when flushing This adds a check for the bug addressed by the previous commit. --- .../Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp | 2 ++ .../Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp index 28276f5fa3..9c6c395a57 100644 --- a/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp +++ b/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp @@ -421,7 +421,9 @@ void RegCache::Flush(BitSet32 pregs) switch (m_regs[i].GetLocationType()) { case PPCCachedReg::LocationType::Default: + break; case PPCCachedReg::LocationType::Discarded: + ASSERT_MSG(DYNA_REC, false, "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/JitArm64/JitArm64_RegCache.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp index 83bbfe1b3a..a1400a5c9a 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp @@ -245,11 +245,14 @@ void Arm64GPRCache::FlushRegisters(BitSet32 regs, bool maintain_state, ARM64Reg { if (regs[i]) { + ASSERT_MSG(DYNA_REC, m_guest_registers[GUEST_GPR_OFFSET + i].GetType() != RegType::Discarded, + "Attempted to flush discarded register"); + if (i + 1 < GUEST_GPR_COUNT && regs[i + 1]) { // We've got two guest registers in a row to store - OpArg& reg1 = m_guest_registers[i]; - OpArg& reg2 = m_guest_registers[i + 1]; + OpArg& reg1 = m_guest_registers[GUEST_GPR_OFFSET + i]; + OpArg& reg2 = m_guest_registers[GUEST_GPR_OFFSET + i + 1]; if (reg1.IsDirty() && reg2.IsDirty() && reg1.GetType() == RegType::Register && reg2.GetType() == RegType::Register) { @@ -283,6 +286,9 @@ void Arm64GPRCache::FlushCRRegisters(BitSet32 regs, bool maintain_state, ARM64Re { if (regs[i]) { + ASSERT_MSG(DYNA_REC, m_guest_registers[GUEST_CR_OFFSET + i].GetType() != RegType::Discarded, + "Attempted to flush discarded register"); + FlushRegister(GUEST_CR_OFFSET + i, maintain_state, tmp_reg); } }