From 411ebadc73e390a83715d9dc337cc8e052516b0d Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 6 Jan 2025 09:36:01 +0100 Subject: [PATCH] JitArm64: Fix crandc/creqv/crorc setting eq bit When I wrote 71e9766519, there was an interaction I didn't take into account: When setting eq, SetCRFieldBit assumes that all bits in the passed-in host register except the least significant bit are 0. But if we use BIC/EON/ORN, all bits except the least significant bit get set to 1. This can cause eq to end up unset when it should be set. This commit fixes the issue. Note that in practice, we never have both bits_1_to_31_are_set and negate at once, so while it looks like this commit adds an extra AND instruction in some cases, those cases don't happen in practice, meaning this fix shouldn't affect performance. --- Source/Core/Core/PowerPC/JitArm64/Jit.h | 5 ++++- .../JitArm64/JitArm64_SystemRegisters.cpp | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.h b/Source/Core/Core/PowerPC/JitArm64/Jit.h index 3f9c471e11c7..fd5cc4e32eb1 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.h +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.h @@ -356,7 +356,10 @@ class JitArm64 : public JitBase, public Arm64Gen::ARM64CodeBlock, public CommonA void WriteBLRExit(Arm64Gen::ARM64Reg dest); void GetCRFieldBit(int field, int bit, Arm64Gen::ARM64Reg out); - void SetCRFieldBit(int field, int bit, Arm64Gen::ARM64Reg in, bool negate = false); + // This assumes that all bits except for bit 0 (LSB) are set to 0. But if bits_1_to_31_are_set + // equals true, it instead assumes that all of bits 1 to 31 are set. + void SetCRFieldBit(int field, int bit, Arm64Gen::ARM64Reg in, bool negate = false, + bool bits_1_to_31_are_set = false); void ClearCRFieldBit(int field, int bit); void SetCRFieldBit(int field, int bit); void FixGTBeforeSettingCRFieldBit(Arm64Gen::ARM64Reg reg); diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp index a0084953b92a..84ecf0470a0c 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp @@ -50,7 +50,8 @@ void JitArm64::GetCRFieldBit(int field, int bit, ARM64Reg out) } } -void JitArm64::SetCRFieldBit(int field, int bit, ARM64Reg in, bool negate) +void JitArm64::SetCRFieldBit(int field, int bit, ARM64Reg in, bool negate, + bool bits_1_to_31_are_set) { gpr.BindCRToRegister(field, true); ARM64Reg CR = gpr.CR(field); @@ -70,7 +71,9 @@ void JitArm64::SetCRFieldBit(int field, int bit, ARM64Reg in, bool negate) AND(CR, CR, LogicalImm(0xFFFF'FFFF'0000'0000, GPRSize::B64)); ORR(CR, CR, in); if (!negate) - EOR(CR, CR, LogicalImm(1ULL << 0, GPRSize::B64)); + EOR(CR, CR, LogicalImm(bits_1_to_31_are_set ? 0xFFFF'FFFFULL : 1ULL, GPRSize::B64)); + else if (bits_1_to_31_are_set) + AND(CR, CR, LogicalImm(0xFFFF'FFFF'0000'0001ULL, GPRSize::B64)); break; case PowerPC::CR_GT_BIT: // set bit 63 to !input @@ -634,6 +637,7 @@ void JitArm64::crXXX(UGeckoInstruction inst) // crnor or crnand const bool negate_result = inst.SUBOP10 == 33 || inst.SUBOP10 == 225; + bool bits_1_to_31_are_set = false; auto WA = gpr.GetScopedReg(); ARM64Reg XA = EncodeRegTo64(WA); @@ -653,7 +657,8 @@ void JitArm64::crXXX(UGeckoInstruction inst) break; case 129: // crandc: A && ~B - BIC(XA, XA, XB); + BIC(WA, WA, WB); + bits_1_to_31_are_set = true; break; case 193: // crxor: A ^ B @@ -661,7 +666,8 @@ void JitArm64::crXXX(UGeckoInstruction inst) break; case 289: // creqv: ~(A ^ B) = A ^ ~B - EON(XA, XA, XB); + EON(WA, WA, WB); + bits_1_to_31_are_set = true; break; case 33: // crnor: ~(A || B) @@ -670,13 +676,14 @@ void JitArm64::crXXX(UGeckoInstruction inst) break; case 417: // crorc: A || ~B - ORN(XA, XA, XB); + ORN(WA, WA, WB); + bits_1_to_31_are_set = true; break; } } // Store result bit in CRBD - SetCRFieldBit(inst.CRBD >> 2, 3 - (inst.CRBD & 3), XA, negate_result); + SetCRFieldBit(inst.CRBD >> 2, 3 - (inst.CRBD & 3), XA, negate_result, bits_1_to_31_are_set); } void JitArm64::mfcr(UGeckoInstruction inst)