Skip to content

Commit

Permalink
JitArm64: Fix crandc/creqv/crorc setting eq bit
Browse files Browse the repository at this point in the history
When I wrote 71e9766, 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.
  • Loading branch information
JosJuice committed Jan 6, 2025
1 parent 9b3b6be commit c04ef2e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
5 changes: 4 additions & 1 deletion Source/Core/Core/PowerPC/JitArm64/Jit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 12 additions & 5 deletions Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -653,15 +657,17 @@ 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
EOR(XA, XA, XB);
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)
Expand All @@ -670,7 +676,8 @@ 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;
}
}
Expand Down

0 comments on commit c04ef2e

Please sign in to comment.