Fix 8/16-bit ADC/SBC

FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp

@@ -340,13 +340,19 @@ OrderedNode* OpDispatchBuilder::CalculateFlags_ADC(uint8_t SrcSize, OrderedNode*
     HandleNZCV_RMW();
     Res = _AdcWithFlags(OpSize, Src1, Src2);
   } else {
-    auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_RAW_LOC);
-    Res = _Adc(OpSize, Src1, Src2);
+    // Need to zero-extend for correct comparisons below
+    Src2 = _Bfe(OpSize, SrcSize * 8, 0, Src2);
+
+    // Note that we do not extend Src2PlusCF, since we depend on proper
+    // 32-bit arithmetic to correctly handle the Src2 = 0xffff case.
+    OrderedNode* Src2PlusCF = _Adc(OpSize, _Constant(0), Src2);
+
+    // Need to zero-extend for the comparison.
+    Res = _Add(OpSize, Src1, Src2PlusCF);
     Res = _Bfe(OpSize, SrcSize * 8, 0, Res);
 
-    auto SelectOpLT = _Select(FEXCore::IR::COND_ULT, Res, Src2, One, Zero);
-    auto SelectOpLE = _Select(FEXCore::IR::COND_ULE, Res, Src2, One, Zero);
-    auto SelectCF = _Select(FEXCore::IR::COND_EQ, CF, One, SelectOpLE, SelectOpLT);
+    // TODO: We can fold that second Bfe in (cmp uxth).
+    auto SelectCF = _Select(FEXCore::IR::COND_ULT, Res, Src2PlusCF, One, Zero);
 
     SetNZ_ZeroCV(SrcSize, Res);
     SetRFLAG<FEXCore::X86State::RFLAG_CF_RAW_LOC>(SelectCF);
@@ -375,13 +381,17 @@ OrderedNode* OpDispatchBuilder::CalculateFlags_SBB(uint8_t SrcSize, OrderedNode*
     // Rectify output carry
     CarryInvert();
   } else {
+    // Zero extend for correct comparison behaviour with Src1 = 0xffff.
+    Src1 = _Bfe(OpSize, SrcSize * 8, 0, Src1);
+
     auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_RAW_LOC);
-    Res = _Sub(OpSize, Src1, _Add(OpSize, Src2, CF));
+    auto Src1MinusCF = _Sub(OpSize, Src1, CF);
+
+    Res = _Sub(OpSize, Src1MinusCF, Src2);
     Res = _Bfe(OpSize, SrcSize * 8, 0, Res);
 
-    auto SelectOpLT = _Select(FEXCore::IR::COND_UGT, Res, Src1, One, Zero);
-    auto SelectOpLE = _Select(FEXCore::IR::COND_UGE, Res, Src1, One, Zero);
-    auto SelectCF = _Select(FEXCore::IR::COND_EQ, CF, One, SelectOpLE, SelectOpLT);
+    // Need to zero-extend for correct comparisons below
+    auto SelectCF = _Select(FEXCore::IR::COND_ULT, Src1MinusCF, Res, One, Zero);
 
     SetNZ_ZeroCV(SrcSize, Res);
     SetRFLAG<FEXCore::X86State::RFLAG_CF_RAW_LOC>(SelectCF);

unittests/ASM/FEX_bugs/add_sub_carry.asm

@@ -0,0 +1,29 @@
+%ifdef CONFIG
+{
+  "RegData": {
+    "RAX": "0xaeadacac9a9a41e5",
+    "RBX": "0x6162636520238df8"
+  }
+}
+%endif
+
+; FEX had a bug with smaller than 32-bit operations corrupting sbb and adc results.
+; A small test that tests both sbb and adc to ensure it returns data correctly.
+; This was noticed in Final Fantasy 7 (steamid 39140) having broken rendering on the title screen.
+mov rax, 0x4142434445464748
+mov rbx, 0x5152535455565758
+mov rcx, 0x6162636465666768
+
+clc
+sbb al, bl
+sbb ax, bx
+sbb eax, ebx
+sbb rax, rbx
+
+clc
+adc bl, cl
+adc bx, cx
+adc ebx, ecx
+adc rbx, rcx
+
+hlt

unittests/ASM/FEX_bugs/add_sub_carry_2.asm

@@ -0,0 +1,167 @@
+%ifdef CONFIG
+{
+  "RegData": {
+    "RAX": "0xedededee26260e6c",
+    "RBX": "0x121212129498c16d"
+  }
+}
+%endif
+
+; FEX had a bug with smaller than 32-bit operations corrupting sbb and adc results.
+; A small test that tests both sbb and adc to ensure it returns data correctly.
+; This was noticed in Final Fantasy 7 (steamid 39140) having broken rendering on the title screen.
+mov rax, 0x4142434445464748
+mov rbx, 0x5152535455565758
+mov rcx, 0x6162636465666768
+
+clc
+sbb al, bl
+sbb ax, bx
+sbb eax, ebx
+sbb rax, rbx
+
+%assign i 0
+%rep 256
+sbb al, [rel .data1 + i]
+%assign i i+1
+%endrep
+
+%assign i 0
+%rep 256
+sbb ax, [rel .data2 + i]
+%assign i i+1
+%endrep
+
+%assign i 0
+%rep 256
+sbb eax, [rel .data4 + i]
+%assign i i+1
+%endrep
+
+
+%assign i 0
+%rep 256
+sbb rax, [rel .data8 + i]
+%assign i i+1
+%endrep
+
+stc
+%assign i 0
+%rep 256
+sbb al, [rel .data1 + i]
+%assign i i+1
+%endrep
+
+%assign i 0
+%rep 256
+sbb ax, [rel .data2 + i]
+%assign i i+1
+%endrep
+
+%assign i 0
+%rep 256
+sbb eax, [rel .data4 + i]
+%assign i i+1
+%endrep
+
+
+%assign i 0
+%rep 256
+sbb rax, [rel .data8 + i]
+%assign i i+1
+%endrep
+
+
+
+
+clc
+adc bl, cl
+adc bx, cx
+adc ebx, ecx
+adc rbx, rcx
+
+
+%assign i 0
+%rep 256
+adc bl, [rel .data1 + i]
+%assign i i+1
+%endrep
+
+%assign i 0
+%rep 256
+adc bx, [rel .data2 + i]
+%assign i i+1
+%endrep
+
+%assign i 0
+%rep 256
+adc ebx, [rel .data4 + i]
+%assign i i+1
+%endrep
+
+
+%assign i 0
+%rep 256
+adc rbx, [rel .data8 + i]
+%assign i i+1
+%endrep
+
+
+stc
+%assign i 0
+%rep 256
+adc bl, [rel .data1 + i]
+%assign i i+1
+%endrep
+
+%assign i 0
+%rep 256
+adc bx, [rel .data2 + i]
+%assign i i+1
+%endrep
+
+%assign i 0
+%rep 256
+adc ebx, [rel .data4 + i]
+%assign i i+1
+%endrep
+
+
+%assign i 0
+%rep 256
+adc rbx, [rel .data8 + i]
+%assign i i+1
+%endrep
+
+
+
+
+hlt
+
+.data1:
+%assign i 0
+%rep 256
+db i
+%assign i i+1
+%endrep
+
+.data2:
+%assign i 0
+%rep 256
+dw i
+%assign i i+1
+%endrep
+
+.data4:
+%assign i 0
+%rep 256
+dd i
+%assign i i+1
+%endrep
+
+.data8:
+%assign i 0
+%rep 256
+dq i
+%assign i i+1
+%endrep