diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index 51b305b65..20de85532 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -1526,7 +1526,7 @@ void OpDispatchBuilder::SHLDImmediateOp(OpcodeArgs) { Res = _Extr(OpSizeFromSrc(Op), Dest, Src, Size - Shift); } - CalculateFlags_ShiftLeftImmediate(OpSizeFromSrc(Op), Res, Dest, Shift); + CalculateFlags_ShiftLeftImmediate(OpSizeFromSrc(Op), Res, Dest, Shift, true); CalculateDeferredFlags(); StoreResultGPR(Op, Res); } else if (Shift == 0 && Size == 32) { diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h index 0b2b64083..416bec3b4 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h @@ -2375,7 +2375,7 @@ private: void CalculateFlags_MUL(IR::OpSize SrcSize, Ref Res, Ref High); void CalculateFlags_UMUL(Ref High); void CalculateFlags_Logical(IR::OpSize SrcSize, Ref Res); - void CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift); + void CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift, bool DoubleWide = false); void CalculateFlags_ShiftRightImmediate(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift); void CalculateFlags_ShiftRightDoubleImmediate(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift); void CalculateFlags_ShiftRightImmediateCommon(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift); diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp index 82c1a1a6c..858e7a4ae 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp @@ -432,7 +432,7 @@ void OpDispatchBuilder::CalculateFlags_Logical(IR::OpSize SrcSize, Ref Res) { SetNZP_ZeroCV(SrcSize, Res); } -void OpDispatchBuilder::CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Ref UnmaskedRes, Ref Src1, uint64_t Shift) { +void OpDispatchBuilder::CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Ref UnmaskedRes, Ref Src1, uint64_t Shift, bool DoubleWide) { // No flags changed if shift is zero if (Shift == 0) { return; @@ -447,8 +447,12 @@ void OpDispatchBuilder::CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Re // Extract the last bit shifted in to CF. Shift is already masked, but for // 8/16-bit it might be >= SrcSizeBits, in which case CF is cleared. There's // nothing to do in that case since we already cleared CF above. + // + // - Double-wide shift has UB when shift is GREATER-THAN operand. + // - Single-wide shift has UB when shift is GREATER-THAN-EQUAL operand. const auto SrcSizeBits = IR::OpSizeAsBits(SrcSize); - if (Shift < SrcSizeBits) { + const bool ShouldSetCF = DoubleWide ? (Shift <= SrcSizeBits) : (Shift < SrcSizeBits); + if (ShouldSetCF) { SetCFDirect(Src1, SrcSizeBits - Shift, true); } } diff --git a/unittests/ASM/FEX_bugs/SHLD_CF.asm b/unittests/ASM/FEX_bugs/SHLD_CF.asm new file mode 100644 index 000000000..9575ccb8c --- /dev/null +++ b/unittests/ASM/FEX_bugs/SHLD_CF.asm @@ -0,0 +1,52 @@ +%ifdef CONFIG +{ + "RegData": { + "RAX": "1", + "R15": "1", + "R14": "1", + "R13": "1", + "R12": "1" + } +} +%endif + +; FEX-Emu had a bug where operand-sized shifts with SHLD/SHRD wasn't matching behaviour. +; We were expecting SAL/SAR/SHL/SHR undefined behaviour semantics but a `ge` comparison changes to `gt`. + +; Test with shld and immediate +mov eax, 1 +mov edx, 0xbee8 +mov rcx, 0 +clc +shld ax, dx, 16 +setc cl +mov r15, rcx + +; Test with shld and CL +mov eax, 1 +mov edx, 0xbee8 +mov rcx, 16 +clc +shld ax, dx, cl +setc cl +mov r14, rcx + +; Test with shrd and immediate +mov eax, 0x8eeb +mov edx, 1 +mov rcx, 0 +clc +shrd ax, dx, 16 +setc cl +mov r13, rcx + +; Test with shrd and CL +mov eax, 0x8eeb +mov edx, 1 +mov rcx, 16 +clc +shrd ax, dx, cl +setc cl +mov r12, rcx + +hlt