From 61d2a09827c85e37546eed0104353b5f76f72cbb Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 18 Mar 2025 15:04:34 -0400 Subject: [PATCH] OpcodeDispatcher: fix BT flags ZF needs to be preserved. the new code is the same instr count on flagm although probably an extra uop. the inst count regression is on flagm, but we can't tolerate broken behaviour. Signed-off-by: Alyssa Rosenzweig --- .../Interface/Core/OpcodeDispatcher.cpp | 12 ++++++++--- .../Source/Interface/Core/OpcodeDispatcher.h | 21 ------------------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index c76c39acd..5900f0594 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -2400,7 +2400,10 @@ void OpDispatchBuilder::BTOp(OpcodeArgs, uint32_t SrcIndex, BTAction Action) { unsigned LshrSize = std::max(IR::OpSizeToSize(OpSize::i32Bit), Size / 8); auto BitSelect = (Size == (LshrSize * 8)) ? Src : _And(OpSize::i64Bit, Src, _Constant(Mask)); - // OF/SF/ZF/AF/PF undefined. + // OF/SF/AF/PF undefined. ZF must be preserved. We choose to preserve OF/SF + // too since we just use an rmif to insert into CF directly. We could + // optimize perhaps. + // // Set CF before the action to save a move, except for complements where we // can reuse the invert. if (Action != BTAction::BTComplement) { @@ -2408,7 +2411,8 @@ void OpDispatchBuilder::BTOp(OpcodeArgs, uint32_t SrcIndex, BTAction Action) { Value = _Lshr(IR::SizeToOpSize(LshrSize), Value, BitSelect); } - SetCFDirect_InvalidateNZV(Value, ConstantShift, Value); + SetRFLAG(Value, X86State::RFLAG_CF_RAW_LOC, ConstantShift, true); + CFInverted = false; } switch (Action) { @@ -2441,7 +2445,9 @@ void OpDispatchBuilder::BTOp(OpcodeArgs, uint32_t SrcIndex, BTAction Action) { Value = Dest; } - SetCFInverted_InvalidateNZV(Value, ConstantShift, true); + SetRFLAG(Value, X86State::RFLAG_CF_RAW_LOC, ConstantShift, true); + CFInverted = true; + StoreResult(GPRClass, Op, Dest, OpSize::iInvalid); break; } diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h index 4372b0592..1f71c34e9 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h @@ -1735,27 +1735,6 @@ private: CFInverted = true; } - // As above but with - // - // x - 1 - // - // If x = 0, hardware C is not set. If x = 1, hardware C is set. - void SetCFInverted_InvalidateNZV(Ref Value, unsigned ValueOffset = 0, bool MustMask = false) { - if (CTX->HostFeatures.SupportsFlagM) { - // This turns into a single rmif - SetCFInverted(Value, ValueOffset, MustMask); - } else { - // Do math on flagm - if (ValueOffset || MustMask) { - Value = _Bfe(OpSize::i64Bit, 1, ValueOffset, Value); - } - - HandleNZCVWrite(); - _SubNZCV(OpSize::i32Bit, Value, _InlineConstant(1)); - CFInverted = true; - } - } - void SetCFInverted(Ref Value, unsigned ValueOffset = 0, bool MustMask = false) { SetRFLAG(Value, X86State::RFLAG_CF_RAW_LOC, ValueOffset, MustMask); CFInverted = true;