Merge pull request #14198 from JosJuice/jit64-cmpxx-rb-comparand

Jit64: Improve cmpXX imm handling

Source/Core/Core/PowerPC/Jit64/Jit_Integer.cpp

@@ -541,39 +541,38 @@ void Jit64::cmpXX(UGeckoInstruction inst)
   u32 crf = inst.CRFD;
   bool merge_branch = CheckMergedBranch(crf);
 
-  bool signedCompare;
+  bool signedCompare = false;
-  RCOpArg comparand;
+  std::optional<u32> imm_comparand;
   switch (inst.OPCD)
   {
   // cmp / cmpl
   case 31:
     signedCompare = (inst.SUBOP10 == 0);
-    comparand = signedCompare ? gpr.Use(b, RCMode::Read) : gpr.Bind(b, RCMode::Read);
+    if (gpr.IsImm(b))
-    RegCache::Realize(comparand);
+      imm_comparand = gpr.Imm32(b);
     break;
 
   // cmpli
   case 10:
     signedCompare = false;
-    comparand = RCOpArg::Imm32((u32)inst.UIMM);
+    imm_comparand = u32(inst.UIMM);
     break;
 
   // cmpi
   case 11:
     signedCompare = true;
-    comparand = RCOpArg::Imm32((u32)(s32)(s16)inst.UIMM);
+    imm_comparand = u32(s32(s16(inst.UIMM)));
     break;
 
   default:
-    signedCompare = false;  // silence compiler warning
     PanicAlertFmt("cmpXX");
   }
 
-  if (gpr.IsImm(a) && comparand.IsImm())
+  if (gpr.IsImm(a) && imm_comparand.has_value())
   {
     // Both registers contain immediate values, so we can pre-compile the compare result
-    s64 compareResult = signedCompare ? (s64)gpr.SImm32(a) - (s64)comparand.SImm32() :
+    s64 compareResult = signedCompare ? s64(gpr.SImm32(a)) - s64(s32(imm_comparand.value())) :
-                                        (u64)gpr.Imm32(a) - (u64)comparand.Imm32();
+                                        u64(gpr.Imm32(a)) - u64(imm_comparand.value());
     if (compareResult == (s32)compareResult)
     {
       MOV(64, PPCSTATE_CR(crf), Imm32((u32)compareResult));
@@ -585,15 +584,12 @@ void Jit64::cmpXX(UGeckoInstruction inst)
     }
 
     if (merge_branch)
-    {
-      RegCache::Unlock(comparand);
       DoMergedBranchImmediate(compareResult);
-    }
 
     return;
   }
 
-  if (!gpr.IsImm(a) && !signedCompare && comparand.IsImm() && comparand.Imm32() == 0)
+  if (!gpr.IsImm(a) && !signedCompare && imm_comparand == u32(0))
   {
     RCX64Reg Ra = gpr.Bind(a, RCMode::Read);
     RegCache::Realize(Ra);
@@ -602,12 +598,38 @@ void Jit64::cmpXX(UGeckoInstruction inst)
     if (merge_branch)
     {
       TEST(64, Ra, Ra);
-      RegCache::Unlock(comparand, Ra);
+      RegCache::Unlock(Ra);
       DoMergedBranchCondition();
     }
     return;
   }
 
+  const bool comparand_needs_reg =
+      !signedCompare && imm_comparand.has_value() && (imm_comparand.value() & 0x80000000U) != 0;
+
+  RCOpArg Rb;
+  OpArg comparand;
+  if (inst.OPCD != 31)
+  {
+    // The comparand is from the instruction encoding, so we can't get it from the register cache.
+    comparand = Imm32(imm_comparand.value());
+  }
+  else if (signedCompare && imm_comparand.has_value())
+  {
+    // Asking the register cache for the comparand might give us a register, but we prefer an imm.
+    comparand = Imm32(imm_comparand.value());
+  }
+  else if (imm_comparand != u32(0))
+  {
+    // Ask the register cache for the comparand. We prefer getting a register,
+    // but may also accept imm or memory depending on the circumstances.
+    Rb = signedCompare ?
+             gpr.Use(b, RCMode::Read) :
+             (comparand_needs_reg ? gpr.Bind(b, RCMode::Read) : gpr.BindOrImm(b, RCMode::Read));
+    RegCache::Realize(Rb);
+    comparand = Rb;
+  }
+
   const X64Reg input = RSCRATCH;
   if (gpr.IsImm(a))
   {
@@ -626,25 +648,7 @@ void Jit64::cmpXX(UGeckoInstruction inst)
       MOVZX(64, 32, input, Ra);
   }
 
-  if (comparand.IsImm())
+  if (imm_comparand == u32(0))
-  {
-    // sign extension will ruin this, so store it in a register
-    if (!signedCompare && (comparand.Imm32() & 0x80000000U) != 0)
-    {
-      MOV(32, R(RSCRATCH2), comparand);
-      comparand = RCOpArg::R(RSCRATCH2);
-    }
-  }
-  else
-  {
-    if (signedCompare)
-    {
-      MOVSX(64, 32, RSCRATCH2, comparand);
-      comparand = RCOpArg::R(RSCRATCH2);
-    }
-  }
-
-  if (comparand.IsImm() && comparand.Imm32() == 0)
   {
     MOV(64, PPCSTATE_CR(crf), R(input));
     // Place the comparison next to the branch for macro-op fusion
@@ -653,13 +657,31 @@ void Jit64::cmpXX(UGeckoInstruction inst)
   }
   else
   {
+    if (comparand.IsImm())
+    {
+      if (comparand_needs_reg)
+      {
+        // sign extension will ruin this, so store it in a register
+        MOV(32, R(RSCRATCH2), comparand);
+        comparand = R(RSCRATCH2);
+      }
+    }
+    else
+    {
+      if (signedCompare)
+      {
+        MOVSX(64, 32, RSCRATCH2, comparand);
+        comparand = R(RSCRATCH2);
+      }
+    }
+
     SUB(64, R(input), comparand);
     MOV(64, PPCSTATE_CR(crf), R(input));
   }
 
   if (merge_branch)
   {
-    RegCache::Unlock(comparand);
+    RegCache::Unlock(Rb);
     DoMergedBranchCondition();
   }
 }