diff --git a/FEXCore/Source/Interface/Core/Core.cpp b/FEXCore/Source/Interface/Core/Core.cpp index 6ebd28de8..ff83858e6 100644 --- a/FEXCore/Source/Interface/Core/Core.cpp +++ b/FEXCore/Source/Interface/Core/Core.cpp @@ -252,12 +252,13 @@ namespace FEXCore::Context { // PF calculation is deferred, calculate it now. // Popcount the 8-bit flag and then extract the lower bit. - uint32_t PF = std::popcount(Frame->State.flags[X86State::RFLAG_PF_LOC]) & 1; + uint32_t PFByte = Frame->State.flags[X86State::RFLAG_PF_LOC]; + uint32_t PF = std::popcount(PFByte) & 1; EFLAGS |= PF << X86State::RFLAG_PF_LOC; // AF calculation is deferred, calculate it now. - // extract bit 4. - uint32_t AF = (Frame->State.flags[X86State::RFLAG_AF_LOC] & (1 << 4)) ? 1 : 0; + // XOR with PF byte and extract bit 4. + uint32_t AF = ((Frame->State.flags[X86State::RFLAG_AF_LOC] ^ PFByte) & (1 << 4)) ? 1 : 0; EFLAGS |= AF << X86State::RFLAG_AF_LOC; return EFLAGS; @@ -274,7 +275,9 @@ namespace FEXCore::Context { // Intentionally do nothing. break; case X86State::RFLAG_AF_LOC: - // AF stored in bit 4 in our internal representation + // AF stored in bit 4 in our internal representation. It is also + // XORed with byte 4 of the PF byte, but we write that as zero here so + // we don't need any special handling for that. Frame->State.flags[i] = (EFLAGS & (1U << i)) ? (1 << 4) : 0; break; case X86State::RFLAG_PF_LOC: diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index 6f9f2ba49..fe4cf718a 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -3485,6 +3485,7 @@ void OpDispatchBuilder::DAAOp(OpcodeArgs) { SetRFLAG(_Select(FEXCore::IR::COND_UGE, _And(OpSize::i64Bit, AL, _Constant(0x80)), _Constant(0), _Constant(1), _Constant(0))); SetRFLAG(_Select(FEXCore::IR::COND_EQ, _And(OpSize::i64Bit, AL, _Constant(0xFF)), _Constant(0), _Constant(1), _Constant(0))); CalculatePFUncheckedABI(AL); + FixupAF(); } void OpDispatchBuilder::DASOp(OpcodeArgs) { @@ -3557,6 +3558,7 @@ void OpDispatchBuilder::DASOp(OpcodeArgs) { SetRFLAG(_Select(FEXCore::IR::COND_UGE, _And(OpSize::i64Bit, AL, _Constant(0x80)), _Constant(0), _Constant(1), _Constant(0))); SetRFLAG(_Select(FEXCore::IR::COND_EQ, _And(OpSize::i64Bit, AL, _Constant(0xFF)), _Constant(0), _Constant(1), _Constant(0))); CalculatePFUncheckedABI(AL); + FixupAF(); } void OpDispatchBuilder::AAAOp(OpcodeArgs) { @@ -3653,6 +3655,7 @@ void OpDispatchBuilder::AAMOp(OpcodeArgs) { SetRFLAG(_Select(FEXCore::IR::COND_UGE, _And(OpSize::i64Bit, AL, _Constant(0x80)), _Constant(0), _Constant(1), _Constant(0))); SetRFLAG(_Select(FEXCore::IR::COND_EQ, _And(OpSize::i64Bit, AL, _Constant(0xFF)), _Constant(0), _Constant(1), _Constant(0))); CalculatePFUncheckedABI(AL); + _InvalidateFlags(1u << X86State::RFLAG_AF_LOC); } void OpDispatchBuilder::AADOp(OpcodeArgs) { @@ -3670,6 +3673,7 @@ void OpDispatchBuilder::AADOp(OpcodeArgs) { SetRFLAG(_Select(FEXCore::IR::COND_UGE, _And(OpSize::i64Bit, AL, _Constant(0x80)), _Constant(0), _Constant(1), _Constant(0))); SetRFLAG(_Select(FEXCore::IR::COND_EQ, _And(OpSize::i64Bit, AL, _Constant(0xFF)), _Constant(0), _Constant(1), _Constant(0))); CalculatePFUncheckedABI(AL); + _InvalidateFlags(1u << X86State::RFLAG_AF_LOC); } void OpDispatchBuilder::XLATOp(OpcodeArgs) { diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h index abdae6b62..853dc1270 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h @@ -1378,6 +1378,7 @@ private: * @{ */ OrderedNode *LoadPF(); OrderedNode *LoadAF(); + void FixupAF(); void CalculatePFUncheckedABI(OrderedNode *Res, OrderedNode *condition = nullptr); void CalculatePF(OrderedNode *Res, OrderedNode *condition = nullptr); void CalculateAF(OpSize OpSize, OrderedNode *Res, OrderedNode *Src1, OrderedNode *Src2); diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp index d105b1b76..3cedb92fd 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp @@ -110,6 +110,9 @@ void OpDispatchBuilder::SetPackedRFLAG(bool Lower8, OrderedNode *Src) { if (FlagOffset == FEXCore::X86State::RFLAG_AF_LOC) { // AF is in bit 4 architecturally, and we need to store it to bit 4 of our // AF register, with garbage in the other bits. The extract is deferred. + // We also defer a XOR with the result bit, which is implemented as XOR + // with PF[4]. But the _Bfe below reliably zeros bit 4 of the PF byte, so + // that will be a no-op and we get the right result. // // So we write out the whole flags byte to AF without an extract. static_assert(FEXCore::X86State::RFLAG_AF_LOC == 4); @@ -152,18 +155,16 @@ OrderedNode *OpDispatchBuilder::GetPackedRFLAG(uint32_t FlagsMask) { } // Note that the Bfi only considers the bottom bit of the flag, the rest of - // the byte is allowed to be garbage. AF is intentionally not using LoadAF. - OrderedNode *Flag = FlagOffset == FEXCore::X86State::RFLAG_PF_LOC ? - LoadPF() : - GetRFLAG(FlagOffset); + // the byte is allowed to be garbage. + OrderedNode *Flag; + if (FlagOffset == FEXCore::X86State::RFLAG_PF_LOC) + Flag = LoadPF(); + else if (FlagOffset == FEXCore::X86State::RFLAG_AF_LOC) + Flag = LoadAF(); + else + Flag = GetRFLAG(FlagOffset); - // AF is special, since the flag value is at bit 4 of our AF register, and - // we need to put it at bit 4 too. So instead of the usual Bfe+Orlshl we - // instead do And+Or. This is the same instruction count, but fewer cycles. - if (FlagOffset == FEXCore::X86State::RFLAG_AF_LOC) { - auto MaskedAF = _And(OpSize::i32Bit, Flag, _Constant(1u << 4)); - Original = _Or(OpSize::i32Bit, Original, MaskedAF); - } else if (CTX->BackendFeatures.SupportsShiftedBitwise) { + if (CTX->BackendFeatures.SupportsShiftedBitwise) { Original = _Orlshl(OpSize::i64Bit, Original, Flag, FlagOffset); } else { Original = _Bfi(OpSize::i32Bit, 1, FlagOffset, Original, Flag); @@ -211,11 +212,29 @@ OrderedNode *OpDispatchBuilder::LoadPF() { } OrderedNode *OpDispatchBuilder::LoadAF() { - // Read the stored byte. This is the XOR result. + // Read the stored byte. This is the XOR of the arguments. auto AFByte = GetRFLAG(FEXCore::X86State::RFLAG_AF_LOC); - // What's left is to extract the flag from the XOR. This is the deferred part. - return _Bfe(OpSize::i32Bit, 1, 4, AFByte); + // Read the result ^ 1, stored as the PF byte for deferred PF calculation. + // This is the same as result as far as the extracted bit 4 is concerned. + auto PFByte = GetRFLAG(FEXCore::X86State::RFLAG_PF_LOC); + + // What's left is to XOR and extract. This is the deferred part. + return _Bfe(OpSize::i32Bit, 1, 4, _Xor(OpSize::i32Bit, AFByte, PFByte)); +} + +void OpDispatchBuilder::FixupAF() { + // The caller has set a desired value of AF in AF[4], regardless of the value + // of PF. We need to fixup AF[4] so that we get the right value when we XOR in + // PF[4] later. The easiest solution is to XOR by PF[4], since: + // + // (AF[4] ^ PF[4]) ^ PF[4] = AF[4] + + auto PFByte = GetRFLAG(FEXCore::X86State::RFLAG_PF_LOC); + auto AFByte = GetRFLAG(FEXCore::X86State::RFLAG_AF_LOC); + + OrderedNode *XorRes = _Xor(OpSize::i32Bit, AFByte, PFByte); + SetRFLAG(XorRes); } void OpDispatchBuilder::CalculatePFUncheckedABI(OrderedNode *Res, OrderedNode *condition) { @@ -244,14 +263,18 @@ void OpDispatchBuilder::CalculatePF(OrderedNode *Res, OrderedNode *condition) { if (!CTX->Config.ABINoPF) { CalculatePFUncheckedABI(Res, condition); } else { - _InvalidateFlags(1UL << FEXCore::X86State::RFLAG_PF_LOC); + // Even if we are skipping PF calculation as a speed hack, we still need to + // zero PF[4] for correct AF. I suspect ABINoPF can be removed now that + // we defer the expensive parts of PF calculation anyway. + SetRFLAG(_Constant(0)); } } void OpDispatchBuilder::CalculateAF(OpSize OpSize, OrderedNode *Res, OrderedNode *Src1, OrderedNode *Src2) { - // We store the XOR of the arguments. The AF flag will be available as a bit - // to be extracted from this result. The extraction is deferred until read. - OrderedNode *XorRes = _Xor(OpSize, _Xor(OpSize, Src1, Src2), Res); + // We store the XOR of the arguments. At read time, we XOR with the + // appropriate bit of the result (available as the PF flag) and extract the + // appropriate bit. + OrderedNode *XorRes = _Xor(OpSize, Src1, Src2); SetRFLAG(XorRes); } @@ -986,6 +1009,9 @@ void OpDispatchBuilder::CalculateFlags_FCMP(uint8_t SrcSize, OrderedNode *Res, O SetRFLAG(HostFlag_ZF); SetRFLAG(HostFlag_Unordered); + // Zero AF. Note that we set the PF byte to 0/1 above, so PF[4] is 0 so the + // XOR with PF will have no effect, so setting the AF byte to zero will indeed + // zero AF as intended. uint32_t FlagsMaskToZero = (1U << X86State::RFLAG_AF_LOC) | (1U << X86State::RFLAG_SF_LOC) |