Remove ABINoPF option

Now that PF calculation is deferred, the cost of calculating PF correctly should
be tolerable. Remove the speed hack to skip PF. It's fundamentally broken, and
there are enough broken things in FEX as it is that we don't need to maintain
this one ;-)

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>

FEXCore/Source/Interface/Config/Config.json.in

@@ -404,15 +404,6 @@
           "Hand-written assembly can violate this assumption."
         ]
       },
-      "ABINoPF": {
-        "Type": "bool",
-        "Default": "false",
-        "Desc": [
-          "When enabled enables an optimization around parity flag calculation.",
-          "Removes the calculation of the parity flag from GPR instructions.",
-          "Assuming no uses rely on it"
-        ]
-      },
       "ParanoidTSO": {
         "Type": "bool",
         "Default": "false",

FEXCore/Source/Interface/Context/Context.h

@@ -221,7 +221,6 @@ namespace FEXCore::Context {
       FEX_CONFIG_OPT(TSOEnabled, TSOENABLED);
       FEX_CONFIG_OPT(TSOAutoMigration, TSOAUTOMIGRATION);
       FEX_CONFIG_OPT(ABILocalFlags, ABILOCALFLAGS);
-      FEX_CONFIG_OPT(ABINoPF, ABINOPF);
       FEX_CONFIG_OPT(AOTIRCapture, AOTIRCAPTURE);
       FEX_CONFIG_OPT(AOTIRGenerate, AOTIRGENERATE);
       FEX_CONFIG_OPT(AOTIRLoad, AOTIRLOAD);

FEXCore/Source/Interface/Core/ObjectCache/CodeObjectSerializationConfig.h

@@ -31,9 +31,6 @@ namespace FEXCore::CodeSerialize {
     // ABI local flag unsafe optimization
     unsigned ABILocalFlags : 1;
 
-    // ABI no PF unsafe optimization
-    unsigned ABINoPF : 1;
-
     // Static register allocation enabled
     unsigned SRA : 1;
 
@@ -51,7 +48,7 @@ namespace FEXCore::CodeSerialize {
 
     // Padding to remove uninitialized data warning from asan
     // Shows remaining amount of bits available for config
-    unsigned _Pad : 17;
+    unsigned _Pad : 18;
 
     bool operator==(CodeObjectSerializationConfig const &other) const {
       return Cookie == other.Cookie &&
@@ -61,7 +58,6 @@ namespace FEXCore::CodeSerialize {
         HardwareTSOEnabled == other.HardwareTSOEnabled &&
         TSOEnabled == other.TSOEnabled &&
         ABILocalFlags == other.ABILocalFlags &&
-        ABINoPF == other.ABINoPF &&
         SRA == other.SRA &&
         ParanoidTSO == other.ParanoidTSO &&
         Is64BitMode == other.Is64BitMode &&
@@ -78,7 +74,6 @@ namespace FEXCore::CodeSerialize {
       Hash <<= 1;  Hash |= other.HardwareTSOEnabled;
       Hash <<= 1;  Hash |= other.TSOEnabled;
       Hash <<= 1;  Hash |= other.ABILocalFlags;
-      Hash <<= 1;  Hash |= other.ABINoPF;
       Hash <<= 1;  Hash |= other.SRA;
       Hash <<= 1;  Hash |= other.ParanoidTSO;
       Hash <<= 1;  Hash |= other.Is64BitMode;

FEXCore/Source/Interface/Core/ObjectCache/NamedRegionObjectHandler.cpp

@@ -17,7 +17,6 @@ namespace FEXCore::CodeSerialize {
     DefaultSerializationConfig.MultiBlock = ctx->Config.Multiblock;
     DefaultSerializationConfig.TSOEnabled = ctx->Config.TSOEnabled;
     DefaultSerializationConfig.ABILocalFlags = ctx->Config.ABILocalFlags;
-    DefaultSerializationConfig.ABINoPF = ctx->Config.ABINoPF;
     DefaultSerializationConfig.SRA = ctx->Config.StaticRegisterAllocation;
     DefaultSerializationConfig.ParanoidTSO = ctx->Config.ParanoidTSO;
     DefaultSerializationConfig.Is64BitMode = ctx->Config.Is64BitMode;

FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp

@@ -3484,7 +3484,7 @@ void OpDispatchBuilder::DAAOp(OpcodeArgs) {
 
   SetRFLAG<FEXCore::X86State::RFLAG_SF_LOC>(_Select(FEXCore::IR::COND_UGE, _And(OpSize::i64Bit, AL, _Constant(0x80)), _Constant(0), _Constant(1), _Constant(0)));
   SetRFLAG<FEXCore::X86State::RFLAG_ZF_LOC>(_Select(FEXCore::IR::COND_EQ, _And(OpSize::i64Bit, AL, _Constant(0xFF)), _Constant(0), _Constant(1), _Constant(0)));
-  CalculatePFUncheckedABI(AL);
+  CalculatePF(AL);
   FixupAF();
 }
 
@@ -3557,7 +3557,7 @@ void OpDispatchBuilder::DASOp(OpcodeArgs) {
   AL = LoadGPRRegister(X86State::REG_RAX, 1);
   SetRFLAG<FEXCore::X86State::RFLAG_SF_LOC>(_Select(FEXCore::IR::COND_UGE, _And(OpSize::i64Bit, AL, _Constant(0x80)), _Constant(0), _Constant(1), _Constant(0)));
   SetRFLAG<FEXCore::X86State::RFLAG_ZF_LOC>(_Select(FEXCore::IR::COND_EQ, _And(OpSize::i64Bit, AL, _Constant(0xFF)), _Constant(0), _Constant(1), _Constant(0)));
-  CalculatePFUncheckedABI(AL);
+  CalculatePF(AL);
   FixupAF();
 }
 
@@ -3654,7 +3654,7 @@ void OpDispatchBuilder::AAMOp(OpcodeArgs) {
   AL = LoadGPRRegister(X86State::REG_RAX, 1);
   SetRFLAG<FEXCore::X86State::RFLAG_SF_LOC>(_Select(FEXCore::IR::COND_UGE, _And(OpSize::i64Bit, AL, _Constant(0x80)), _Constant(0), _Constant(1), _Constant(0)));
   SetRFLAG<FEXCore::X86State::RFLAG_ZF_LOC>(_Select(FEXCore::IR::COND_EQ, _And(OpSize::i64Bit, AL, _Constant(0xFF)), _Constant(0), _Constant(1), _Constant(0)));
-  CalculatePFUncheckedABI(AL);
+  CalculatePF(AL);
   _InvalidateFlags(1u << X86State::RFLAG_AF_LOC);
 }
 
@@ -3672,7 +3672,7 @@ void OpDispatchBuilder::AADOp(OpcodeArgs) {
   AL = LoadGPRRegister(X86State::REG_RAX, 1);
   SetRFLAG<FEXCore::X86State::RFLAG_SF_LOC>(_Select(FEXCore::IR::COND_UGE, _And(OpSize::i64Bit, AL, _Constant(0x80)), _Constant(0), _Constant(1), _Constant(0)));
   SetRFLAG<FEXCore::X86State::RFLAG_ZF_LOC>(_Select(FEXCore::IR::COND_EQ, _And(OpSize::i64Bit, AL, _Constant(0xFF)), _Constant(0), _Constant(1), _Constant(0)));
-  CalculatePFUncheckedABI(AL);
+  CalculatePF(AL);
   _InvalidateFlags(1u << X86State::RFLAG_AF_LOC);
 }
 

FEXCore/Source/Interface/Core/OpcodeDispatcher.h

@@ -1379,7 +1379,6 @@ 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);
 

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

@@ -237,7 +237,7 @@ void OpDispatchBuilder::FixupAF() {
   SetRFLAG<FEXCore::X86State::RFLAG_AF_LOC>(XorRes);
 }
 
-void OpDispatchBuilder::CalculatePFUncheckedABI(OrderedNode *Res, OrderedNode *condition) {
+void OpDispatchBuilder::CalculatePF(OrderedNode *Res, OrderedNode *condition) {
   // We will use the bottom bit of the popcount, set if an odd number of bits are set.
   // But the x86 parity flag is supposed to be set for an even number of bits.
   // Simply invert any bit of the input GPR and that will invert the bottom bit of the
@@ -259,17 +259,6 @@ void OpDispatchBuilder::CalculatePFUncheckedABI(OrderedNode *Res, OrderedNode *c
   SetRFLAG<FEXCore::X86State::RFLAG_PF_LOC>(Flipped);
 }
 
-void OpDispatchBuilder::CalculatePF(OrderedNode *Res, OrderedNode *condition) {
-  if (!CTX->Config.ABINoPF) {
-    CalculatePFUncheckedABI(Res, condition);
-  } else {
-    // 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<FEXCore::X86State::RFLAG_PF_LOC>(_Constant(0));
-  }
-}
-
 void OpDispatchBuilder::CalculateAF(OpSize OpSize, OrderedNode *Res, OrderedNode *Src1, OrderedNode *Src2) {
   // 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

FEXCore/Source/Interface/IR/AOTIR.cpp

@@ -386,13 +386,12 @@ namespace FEXCore::IR {
     if (!base_filename.empty()) {
       auto filename_hash = XXH3_64bits(filename.c_str(), filename.size());
 
-      auto fileid = fextl::fmt::format("{}-{}-{}{}{}{}",
+      auto fileid = fextl::fmt::format("{}-{}-{}{}{}",
         base_filename,
         filename_hash,
         (CTX->Config.SMCChecks == FEXCore::Config::CONFIG_SMC_FULL) ? 'S' : 's',
         CTX->Config.TSOEnabled ? 'T' : 't',
-        CTX->Config.ABILocalFlags ? 'L' : 'l',
-        CTX->Config.ABINoPF ? 'p' : 'P');
+        CTX->Config.ABILocalFlags ? 'L' : 'l');
 
       std::unique_lock lk(AOTIRCacheLock);
 

Scripts/FEXUpdateAOTIRCache.sh

@@ -4,25 +4,19 @@ echo Using $FEX
 for fileid in ~/.fex-emu/aotir/*.path; do
 	filename=`cat "$fileid"`
 	args=""
-	if [ "${fileid: -6 : 1}" == "P" ]; then
-		args="$args --no-abinopf"
-	else
-		args="$args --abinopf"
-	fi
-
-	if [ "${fileid: -7 : 1}" == "L" ]; then
+	if [ "${fileid: -6 : 1}" == "L" ]; then
 		args="$args --abilocalflags"
 	else
 		args="$args --no-abilocalflags"
 	fi
 	
-	if [ "${fileid: -8 : 1}" == "T" ]; then
+	if [ "${fileid: -7 : 1}" == "T" ]; then
 		args="$args --tsoenabled"
 	else
 		args="$args --no-tsoenabled"
 	fi
 	
-	if [ "${fileid: -9 : 1}" == "S" ]; then
+	if [ "${fileid: -8 : 1}" == "S" ]; then
 		args="$args --smc=full"
 	else
 		args="$args --smc=mman"

Source/Tools/FEXConfig/Main.cpp

@@ -614,13 +614,6 @@ namespace {
         ConfigChanged = true;
       }
 
-      Value = LoadedConfig->Get(FEXCore::Config::ConfigOption::CONFIG_ABINOPF);
-      bool NoPFCalculation = Value.has_value() && **Value == "1";
-      if (ImGui::Checkbox("Disable PF calculation", &NoPFCalculation)) {
-        LoadedConfig->EraseSet(FEXCore::Config::ConfigOption::CONFIG_ABINOPF, NoPFCalculation ? "1" : "0");
-        ConfigChanged = true;
-      }
-
       ImGui::EndTabItem();
     }
   }