Merge pull request #5809 from simon902/fcomi_flag_zero

FCOMI fails to clear OF, SF, AF

FEXCore/Source/Interface/Core/OpcodeDispatcher.h

@@ -2131,16 +2131,16 @@ private:
   }
 
   // Compares two floats and sets flags for a COMISS instruction
-  void Comiss(IR::OpSize ElementSize, Ref Src1, Ref Src2, bool InvalidateAF = false) {
+  void Comiss(IR::OpSize ElementSize, Ref Src1, Ref Src2) {
     // First, set flags according to Arm FCMP.
     HandleNZCVWrite();
     _FCmp(ElementSize, Src1, Src2);
     CFInverted = false;
-    ComissFlags(InvalidateAF);
+    ComissFlags();
   }
 
   // Sets flags for a COMISS instruction
-  void ComissFlags(bool InvalidateAF = false) {
+  void ComissFlags() {
     LOGMAN_THROW_A_FMT(!NZCVDirty, "only expected after fcmp");
 
     // We need to set PF according to the unordered flag. We'd rather do this
@@ -2155,12 +2155,15 @@ private:
     Ref V_inv = GetRFLAG(FEXCore::X86State::RFLAG_OF_RAW_LOC, true);
     SetRFLAG<FEXCore::X86State::RFLAG_PF_RAW_LOC>(V_inv);
 
-    if (!InvalidateAF) {
-      // Zero AF. Note that the comparison sets the raw PF 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.
-      SetRFLAG<FEXCore::X86State::RFLAG_AF_RAW_LOC>(Constant(0));
-    }
+    // Intel: OF, SF, and AF set to zero
+    // AMD:   no mention of OF, SF and AF but actual hardware seems to always zero
+    //
+    // Zero AF. Note that the comparison sets the raw PF 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.
+    // OF and SF are zeroed:
+    // _AXFLAG always produces N=0 (SF), V=0 (OF)
+    SetRFLAG<FEXCore::X86State::RFLAG_AF_RAW_LOC>(Constant(0));
 
     // Convert NZCV from the Arm representation to an eXternal representation
     // that's totally not a euphemism for x86, nuh-uh. But maps to exactly we

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

@@ -667,7 +667,6 @@ void OpDispatchBuilder::FCOMI(OpcodeArgs, IR::OpSize Width, bool Integer, OpDisp
     SetRFLAG<FEXCore::X86State::X87FLAG_C2_LOC>(HostFlag_Unordered);
     SetRFLAG<FEXCore::X86State::X87FLAG_C3_LOC>(HostFlag_ZF);
   } else {
-    // OF, SF, AF, PF all undefined
     SetCFDirect(HostFlag_CF);
     SetRFLAG<FEXCore::X86State::RFLAG_ZF_RAW_LOC>(HostFlag_ZF);
 
@@ -675,6 +674,12 @@ void OpDispatchBuilder::FCOMI(OpcodeArgs, IR::OpSize Width, bool Integer, OpDisp
     // TODO: This could perhaps be optimized?
     auto PF = _Xor(OpSize::i32Bit, HostFlag_Unordered, Constant(1));
     SetRFLAG<FEXCore::X86State::RFLAG_PF_RAW_LOC>(PF);
+
+    // Intel: OF, SF, and AF set to zero
+    // AMD:   no mention of OF, SF and AF but actual hardware seems to always zero
+    SetRFLAG<FEXCore::X86State::RFLAG_OF_RAW_LOC>(Constant(0));
+    SetRFLAG<FEXCore::X86State::RFLAG_SF_RAW_LOC>(Constant(0));
+    SetRFLAG<FEXCore::X86State::RFLAG_AF_RAW_LOC>(Constant(0));
   }
 
   // Set Invalid Operation flag when unordered (NaN comparison)

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

@@ -367,7 +367,7 @@ void OpDispatchBuilder::FCOMIF64(OpcodeArgs, IR::OpSize Width, bool Integer, OpD
   } else {
     HandleNZCVWrite();
     _F80CmpValue(b);
-    ComissFlags(true /* InvalidateAF */);
+    ComissFlags();
   }
 
   if (PopTwice) {