[AArch64][GlobalISel] NFC: Refactor emitIntegerCompare

[paquette]

Summary

Simplify emitIntegerCompare and improve comments + asserts.

Mostly making the code a little easier to follow.

Also, this code is only used for G_ICMP. The legalizer ensures that the LHS/RHS for every G_ICMP is either a s32 or s64. So, there's no need to handle anything else. This lets us remove a bunch of checks for whether or not we successfully emitted the compare.

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

if (!VRegAndVal) {
     LHSMI = getDefIgnoringCopies(LHS, MRI);
   }
 
   if (!VRegAndVal || VRegAndVal->Value != 0) {
     // If we can't select a CBZ then emit a cmp + Bcc.
     MachineInstr *Cmp;
     std::tie(Cmp, Pred) = emitIntegerCompare(
         CCMI->getOperand(2), CCMI->getOperand(3), CCMI->getOperand(1), MIB);
-    if (!Cmp)
-      return false;
     const AArch64CC::CondCode CC = changeICMPPredToAArch64CC(Pred);
     MIB.buildInstr(AArch64::Bcc, {}, {}).addImm(CC).addMBB(DestMBB);
     I.eraseFromParent();
     return true;
   }
 
   // Try to emit a TB(N)Z for an eq or ne condition.
   if (tryOptAndIntoCompareBranch(LHSMI, VRegAndVal->Value, Pred, DestMBB,
if (Ty != LLT::scalar(32)) {
       return false;
     }
 
     MachineIRBuilder MIRBuilder(I);
     MachineInstr *Cmp;
     CmpInst::Predicate Pred;
     std::tie(Cmp, Pred) = emitIntegerCompare(I.getOperand(2), I.getOperand(3),
                                              I.getOperand(1), MIRBuilder);
-    if (!Cmp)
-      return false;
     emitCSetForICMP(I.getOperand(0).getReg(), Pred, MIRBuilder);
     I.eraseFromParent();
     return true;
   }
 
   case TargetOpcode::G_FCMP: {
     MachineIRBuilder MIRBuilder(I);
     CmpInst::Predicate Pred =

 
 std::pair<MachineInstr *, CmpInst::Predicate>
 AArch64InstructionSelector::emitIntegerCompare(
     MachineOperand &LHS, MachineOperand &RHS, MachineOperand &Predicate,
     MachineIRBuilder &MIRBuilder) const {
   assert(LHS.isReg() && RHS.isReg() && "Expected LHS and RHS to be registers!");
   assert(Predicate.isPredicate() && "Expected predicate?");
   MachineRegisterInfo &MRI = MIRBuilder.getMF().getRegInfo();
+  LLT CmpTy = MRI.getType(LHS.getReg());
+  assert(!CmpTy.isVector() && "Expected scalar or pointer");
+  unsigned Size = CmpTy.getSizeInBits();
+  assert((Size == 32 || Size == 64) && "Expected a 32-bit or 64-bit LHS/RHS?");
+  auto P = static_cast<CmpInst::Predicate>(Predicate.getPredicate());
 
-  CmpInst::Predicate P = (CmpInst::Predicate)Predicate.getPredicate();
-
-  // Fold the compare if possible.
-  MachineInstr *FoldCmp =
-      tryFoldIntegerCompare(LHS, RHS, Predicate, MIRBuilder);
-  if (FoldCmp)
+  // Fold the compare into a cmn or tst if possible.
+  if (auto FoldCmp = tryFoldIntegerCompare(LHS, RHS, Predicate, MIRBuilder))
     return {FoldCmp, P};
 
-  // Can't fold into a CMN. Just emit a normal compare.
-  unsigned CmpOpc = 0;
-  Register ZReg;
-
-  LLT CmpTy = MRI.getType(LHS.getReg());
-  assert((CmpTy.isScalar() || CmpTy.isPointer()) &&
-         "Expected scalar or pointer");
-  if (CmpTy == LLT::scalar(32)) {
-    CmpOpc = AArch64::SUBSWrr;
-    ZReg = MRI.createVirtualRegister(&AArch64::GPR32RegClass);
-  } else if (CmpTy == LLT::scalar(64) || CmpTy.isPointer()) {
-    CmpOpc = AArch64::SUBSXrr;
-    ZReg = MRI.createVirtualRegister(&AArch64::GPR64RegClass);
-  } else {
-    return {nullptr, CmpInst::Predicate::BAD_ICMP_PREDICATE};
-  }
-
-  // Try to match immediate forms.
-  MachineInstr *ImmedCmp =
-      tryOptArithImmedIntegerCompare(LHS, RHS, P, MIRBuilder);
-  if (ImmedCmp)
+  // Compares need special handling for their shifted/immediate forms. We
+  // may be able to modify the predicate or an illegal constant to perform
+  // some folding.
+  if (auto ImmedCmp = tryOptArithImmedIntegerCompare(LHS, RHS, P, MIRBuilder))
     return {ImmedCmp, P};
-
-  // If we don't have an immediate, we may have a shift which can be folded
-  // into the compare.
-  MachineInstr *ShiftedCmp = tryOptArithShiftedCompare(LHS, RHS, MIRBuilder);
-  if (ShiftedCmp)
+  if (auto ShiftedCmp = tryOptArithShiftedCompare(LHS, RHS, MIRBuilder))
     return {ShiftedCmp, P};
-
-  auto CmpMI =
-      MIRBuilder.buildInstr(CmpOpc, {ZReg}, {LHS.getReg(), RHS.getReg()});
-  // Make sure that we can constrain the compare that we emitted.
+  unsigned CmpOpc = Size == 32 ? AArch64::SUBSWrr : AArch64::SUBSXrr;
+  auto CmpMI = MIRBuilder.buildInstr(
+      CmpOpc, {MRI.cloneVirtualRegister(LHS.getReg())}, {LHS, RHS});
   constrainSelectedInstRegOperands(*CmpMI, TII, TRI, RBI);
   return {&*CmpMI, P};
 }
 
 MachineInstr *AArch64InstructionSelector::emitCSetForFCmp(
     Register Dst, CmpInst::Predicate Pred, MachineIRBuilder &MIRBuilder) const {
   MachineRegisterInfo &MRI = *MIRBuilder.getMRI();
 #ifndef NDEBUG
bool AArch64InstructionSelector::tryOptSelect(MachineInstr &I) const {
   if (CondOpc == TargetOpcode::G_ICMP) {
     MachineInstr *Cmp;
     CmpInst::Predicate Pred;
 
     std::tie(Cmp, Pred) =
         emitIntegerCompare(CondDef->getOperand(2), CondDef->getOperand(3),
                            CondDef->getOperand(1), MIB);
 
-    if (!Cmp) {
-      LLVM_DEBUG(dbgs() << "Couldn't emit compare for select!\n");
-      return false;
-    }
-
     // Have to collect the CondCode after emitIntegerCompare, since it can
     // update the predicate.
     CondCode = changeICMPPredToAArch64CC(Pred);
   } else {
     // Get the condition code for the select.
     CmpInst::Predicate Pred =
         static_cast<CmpInst::Predicate>(CondDef->getOperand(1).getPredicate());
     AArch64CC::CondCode CondCode2;