Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TableGen][RISCV][GlobalISel] Select G_ICMP, G_SELECT, G_PTR_ADD, G_STORE, G_LOAD, G_ZEXTLOAD #67185

Closed
wants to merge 5 commits into from

Conversation

nitinjohnraj
Copy link
Contributor

This series of patches works towards implementing instruction selection for RISCV.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 22, 2023

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-llvm-globalisel

Changes

This series of patches works towards implementing instruction selection for RISCV.


Patch is 65.67 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/67185.diff

10 Files Affected:

  • (modified) llvm/include/llvm/Target/Target.td (+5)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp (+87-2)
  • (modified) llvm/lib/Target/RISCV/RISCVGISel.td (+70)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/cmp32.mir (+801)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/cmp64.mir (+801)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/ptradd32.mir (+50)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/ptradd64.mir (+50)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/select32.mir (+55)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/select64.mir (+55)
  • (modified) llvm/utils/TableGen/InfoByHwMode.cpp (+2)
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index 94a57e5c0f6cffa..e4678d777f1359a 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -58,6 +58,11 @@ class ValueTypeByHwMode<list<HwMode> Ms, list<ValueType> Ts>
   list<ValueType> Objects = Ts;
 }
 
+class PtrValueTypeByHwMode<ValueTypeByHwMode scalar, int addrspace>
+    : HwModeSelect<scalar.Modes>, PtrValueType<ValueType<0, 0>, addrspace> {
+  list<ValueType> Objects = scalar.Objects;
+}
+
 // A class representing the register size, spill size and spill alignment
 // in bits of a register.
 class RegInfo<int RS, int SS, int SA> {
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
index 4c246d7de1da952..ac628396ced9e07 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
@@ -45,10 +45,24 @@ class RISCVInstructionSelector : public InstructionSelector {
   getRegClassForTypeOnBank(LLT Ty, const RegisterBank &RB,
                            bool GetAllRegSet = false) const;
 
+  // tblgen-erated 'select' implementation, used as the initial selector for
+  // the patterns that don't require complex C++.
   bool selectImpl(MachineInstr &I, CodeGenCoverage &CoverageInfo) const;
+
+  // A lowering phase that runs before any selection attempts.
+  // Returns true if the instruction was modified.
+  bool preISelLower(MachineInstr &MI, MachineIRBuilder &MIB,
+                    MachineRegisterInfo &MRI);
+
+  bool replacePtrWithInt(MachineOperand &Op, MachineIRBuilder &MIB,
+                         MachineRegisterInfo &MRI);
+
+  // Custom selection methods
   bool selectCopy(MachineInstr &MI, MachineRegisterInfo &MRI) const;
   bool selectConstant(MachineInstr &MI, MachineIRBuilder &MIB,
                       MachineRegisterInfo &MRI) const;
+  bool selectSelect(MachineInstr &MI, MachineIRBuilder &MIB,
+                    MachineRegisterInfo &MRI) const;
 
   bool earlySelectShift(unsigned Opc, MachineInstr &I, MachineIRBuilder &MIB,
                         const MachineRegisterInfo &MRI);
@@ -58,6 +72,8 @@ class RISCVInstructionSelector : public InstructionSelector {
   // Custom renderers for tablegen
   void renderNegImm(MachineInstrBuilder &MIB, const MachineInstr &MI,
                     int OpIdx) const;
+  void renderImmPlus1(MachineInstrBuilder &MIB, const MachineInstr &MI,
+                      int OpIdx) const;
 
   const RISCVSubtarget &STI;
   const RISCVInstrInfo &TII;
@@ -130,12 +146,14 @@ bool RISCVInstructionSelector::earlySelectShift(
 }
 
 bool RISCVInstructionSelector::select(MachineInstr &MI) {
-  unsigned Opc = MI.getOpcode();
   MachineBasicBlock &MBB = *MI.getParent();
   MachineFunction &MF = *MBB.getParent();
   MachineRegisterInfo &MRI = MF.getRegInfo();
   MachineIRBuilder MIB(MI);
 
+  preISelLower(MI, MIB, MRI);
+  const unsigned Opc = MI.getOpcode();
+
   if (!isPreISelGenericOpcode(Opc)) {
     // Certain non-generic instructions also need some special handling.
     if (MI.isCopy())
@@ -198,13 +216,18 @@ bool RISCVInstructionSelector::select(MachineInstr &MI) {
 
   switch (Opc) {
   case TargetOpcode::G_ANYEXT:
+  case TargetOpcode::G_PTRTOINT:
   case TargetOpcode::G_TRUNC:
     MI.setDesc(TII.get(TargetOpcode::COPY));
-    return true;
+    return selectCopy(MI, MRI);
   case TargetOpcode::G_CONSTANT:
     if (!selectConstant(MI, MIB, MRI))
       return false;
     break;
+  case TargetOpcode::G_SELECT:
+    if (!selectSelect(MI, MIB, MRI))
+      return false;
+    break;
   default:
     return false;
   }
@@ -214,6 +237,41 @@ bool RISCVInstructionSelector::select(MachineInstr &MI) {
   return true;
 }
 
+bool RISCVInstructionSelector::replacePtrWithInt(MachineOperand &Op,
+                                                 MachineIRBuilder &MIB,
+                                                 MachineRegisterInfo &MRI) {
+  assert(Op.isReg() && "Operand is not a register!");
+  Register PtrReg = Op.getReg();
+  assert(MRI.getType(PtrReg).isPointer() && "Operand is not a pointer!");
+
+  const LLT XLenLLT = LLT::scalar(STI.getXLen());
+  auto PtrToInt = MIB.buildPtrToInt(XLenLLT, PtrReg);
+  MRI.setRegBank(PtrToInt.getReg(0), RBI.getRegBank(RISCV::GPRRegBankID));
+  MRI.setType(PtrReg, XLenLLT);
+  Op.setReg(PtrToInt.getReg(0));
+  return select(*PtrToInt);
+}
+
+bool RISCVInstructionSelector::preISelLower(MachineInstr &MI,
+                                            MachineIRBuilder &MIB,
+                                            MachineRegisterInfo &MRI) {
+  switch (MI.getOpcode()) {
+  case TargetOpcode::G_PTR_ADD: {
+    Register DstReg = MI.getOperand(0).getReg();
+    const LLT XLenLLT = LLT::scalar(STI.getXLen());
+
+    replacePtrWithInt(MI.getOperand(1), MIB, MRI);
+    MI.setDesc(TII.get(TargetOpcode::G_ADD));
+    MRI.setType(DstReg, XLenLLT);
+    break;
+  }
+  default:
+    return false;
+  }
+
+  return true;
+}
+
 void RISCVInstructionSelector::renderNegImm(MachineInstrBuilder &MIB,
                                             const MachineInstr &MI,
                                             int OpIdx) const {
@@ -223,6 +281,15 @@ void RISCVInstructionSelector::renderNegImm(MachineInstrBuilder &MIB,
   MIB.addImm(-CstVal);
 }
 
+void RISCVInstructionSelector::renderImmPlus1(MachineInstrBuilder &MIB,
+                                           const MachineInstr &MI,
+                                           int OpIdx) const {
+  assert(MI.getOpcode() == TargetOpcode::G_CONSTANT && OpIdx == -1 &&
+         "Expected G_CONSTANT");
+  int64_t CstVal = MI.getOperand(1).getCImm()->getSExtValue();
+  MIB.addImm(CstVal + 1);
+}
+
 const TargetRegisterClass *RISCVInstructionSelector::getRegClassForTypeOnBank(
     LLT Ty, const RegisterBank &RB, bool GetAllRegSet) const {
   if (RB.getID() == RISCV::GPRRegBankID) {
@@ -327,6 +394,24 @@ bool RISCVInstructionSelector::selectConstant(MachineInstr &MI,
   return true;
 }
 
+bool RISCVInstructionSelector::selectSelect(MachineInstr &MI,
+                                            MachineIRBuilder &MIB,
+                                            MachineRegisterInfo &MRI) const {
+  assert(MI.getOpcode() == TargetOpcode::G_SELECT);
+  Register DstReg = MI.getOperand(0).getReg();
+  Register CondReg = MI.getOperand(1).getReg();
+  Register TruReg = MI.getOperand(2).getReg();
+  Register FlsReg = MI.getOperand(3).getReg();
+  MachineInstr *Result = MIB.buildInstr(RISCV::Select_GPR_Using_CC_GPR)
+                             .addDef(DstReg)
+                             .addReg(CondReg)
+                             .addReg(RISCV::X0)
+                             .addImm(RISCVCC::COND_NE)
+                             .addReg(TruReg)
+                             .addReg(FlsReg);
+  return constrainSelectedInstRegOperands(*Result, TII, TRI, RBI);
+}
+
 namespace llvm {
 InstructionSelector *
 createRISCVInstructionSelector(const RISCVTargetMachine &TM,
diff --git a/llvm/lib/Target/RISCV/RISCVGISel.td b/llvm/lib/Target/RISCV/RISCVGISel.td
index 070bb3b42a07c54..cfb0a8d89d781f4 100644
--- a/llvm/lib/Target/RISCV/RISCVGISel.td
+++ b/llvm/lib/Target/RISCV/RISCVGISel.td
@@ -19,9 +19,26 @@ include "RISCVCombine.td"
 def simm12Plus1 : ImmLeaf<XLenVT, [{
     return (isInt<12>(Imm) && Imm != -2048) || Imm == 2048;}]>;
 
+// FIXME: This doesn't check that the G_CONSTANT we're deriving the immediate
+// from is only used once
+def simm12Minus1Nonzero : ImmLeaf<XLenVT, [{
+  return (Imm >= -2049 && Imm < 0) || (Imm > 0 && Imm <= 2046);}]>;
+
+def simm12Minus1NonzeroNonNeg1 : ImmLeaf<XLenVT, [{
+  return (Imm >= -2049 && Imm < -1) || (Imm > 0 && Imm <= 2046);}]>;
+
+// Return an immediate value plus 1.
+def ImmPlus1 : SDNodeXForm<imm, [{
+  return CurDAG->getTargetConstant(N->getSExtValue() + 1, SDLoc(N),
+                                   N->getValuePtrVTpe(0));}]>;
+
 def GINegImm : GICustomOperandRenderer<"renderNegImm">,
   GISDNodeXFormEquiv<NegImm>;
 
+def GIImmPlus1 :
+  GICustomOperandRenderer<"renderImmPlus1">,
+  GISDNodeXFormEquiv<ImmPlus1>;
+
 // FIXME: This is labelled as handling 's32', however the ComplexPattern it
 // refers to handles both i32 and i64 based on the HwMode. Currently this LLT
 // parameter appears to be ignored so this pattern works for both, however we
@@ -54,3 +71,56 @@ def : Pat<(i32 (srem GPR:$rs1, GPR:$rs2)), (REMW GPR:$rs1, GPR:$rs2)>;
 def : Pat<(i32 (udiv GPR:$rs1, GPR:$rs2)), (DIVUW GPR:$rs1, GPR:$rs2)>;
 def : Pat<(i32 (urem GPR:$rs1, GPR:$rs2)), (REMUW GPR:$rs1, GPR:$rs2)>;
 }
+
+// Ptr type used in patterns with GlobalISelEmitter
+def PtrVT : PtrValueTypeByHwMode<XLenVT, 0>;
+
+// Define pattern expansions for pointer ult/slt conditional codes
+def : Pat<(XLenVT (setult (PtrVT GPR:$rs1), simm12:$imm12)),
+          (SLTIU GPR:$rs1, simm12:$imm12)>;
+def : Pat<(XLenVT (setult (PtrVT GPR:$rs1), (PtrVT GPR:$rs2))),
+          (SLTU GPR:$rs1, GPR:$rs2)>;
+def : Pat<(XLenVT (setlt (PtrVT GPR:$rs1), simm12:$imm12)),
+          (SLTI GPR:$rs1, simm12:$imm12)>;
+def : Pat<(XLenVT (setlt (PtrVT GPR:$rs1), (PtrVT GPR:$rs2))),
+          (SLT GPR:$rs1, GPR:$rs2)>;
+
+// Define pattern expansions for setcc operations that aren't directly
+// handled by a RISC-V instruction.
+foreach Ty = [PtrVT, XLenVT] in {
+def : Pat<(XLenVT (seteq (Ty GPR:$rs1), 0)), (SLTIU GPR:$rs1, 1)>;
+def : Pat<(XLenVT (seteq (Ty GPR:$rs1), simm12Plus1:$imm12)),
+          (SLTIU (ADDI GPR:$rs1, (NegImm simm12Plus1:$imm12)), 1)>;
+def : Pat<(XLenVT (seteq (Ty GPR:$rs1), (Ty GPR:$rs2))),
+          (SLTIU (XOR GPR:$rs1, GPR:$rs2), 1)>;
+def : Pat<(XLenVT (setne (Ty GPR:$rs1), 0)), (SLTU (XLenVT X0), GPR:$rs1)>;
+def : Pat<(XLenVT (setne (Ty GPR:$rs1), simm12Plus1:$imm12)),
+          (SLTU (XLenVT X0), (ADDI GPR:$rs1, (NegImm simm12Plus1:$imm12)))>;
+def : Pat<(XLenVT (setne (Ty GPR:$rs1), (Ty GPR:$rs2))),
+          (SLTU (XLenVT X0), (XOR GPR:$rs1, GPR:$rs2))>;
+def : Pat<(XLenVT (setugt (Ty GPR:$rs1), simm12Minus1NonzeroNonNeg1:$imm)),
+          (XORI (SLTIU GPR:$rs1,
+                       (ImmPlus1 simm12Minus1NonzeroNonNeg1:$imm)), 1)>;
+def : Pat<(XLenVT (setugt (Ty GPR:$rs1), (Ty GPR:$rs2))),
+          (SLTU GPR:$rs2, GPR:$rs1)>;
+def : Pat<(XLenVT (setgt (Ty GPR:$rs1), simm12Minus1Nonzero:$imm)),
+          (XORI (SLTI GPR:$rs1, (ImmPlus1 simm12Minus1Nonzero:$imm)), 1)>;
+def : Pat<(XLenVT (setgt (Ty GPR:$rs1), (Ty GPR:$rs2))),
+          (SLT GPR:$rs2, GPR:$rs1)>;
+def : Pat<(XLenVT (setuge (XLenVT GPR:$rs1), simm12:$imm)),
+          (XORI (SLTIU GPR:$rs1, simm12:$imm), 1)>;
+def : Pat<(XLenVT (setuge (Ty GPR:$rs1), (Ty GPR:$rs2))),
+          (XORI (SLTU GPR:$rs1, GPR:$rs2), 1)>;
+def : Pat<(XLenVT (setge (Ty GPR:$rs1), simm12:$imm)),
+          (XORI (SLTI GPR:$rs1, simm12:$imm), 1)>;
+def : Pat<(XLenVT (setge (Ty GPR:$rs1), (Ty GPR:$rs2))),
+          (XORI (SLT GPR:$rs1, GPR:$rs2), 1)>;
+def : Pat<(XLenVT (setule (Ty GPR:$rs1), simm12Minus1NonzeroNonNeg1:$imm)),
+          (SLTIU GPR:$rs1, (ImmPlus1 simm12Minus1NonzeroNonNeg1:$imm))>;
+def : Pat<(XLenVT (setule (Ty GPR:$rs1), (Ty GPR:$rs2))),
+          (XORI (SLTU GPR:$rs2, GPR:$rs1), 1)>;
+def : Pat<(XLenVT (setle (Ty GPR:$rs1), simm12Minus1Nonzero:$imm)),
+          (SLTI GPR:$rs1, (ImmPlus1 simm12Minus1Nonzero:$imm))>;
+def : Pat<(XLenVT (setle (Ty GPR:$rs1), (Ty GPR:$rs2))),
+          (XORI (SLT GPR:$rs2, GPR:$rs1), 1)>;
+}
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/cmp32.mir b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/cmp32.mir
new file mode 100644
index 000000000000000..3ae16181e5fac2c
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/cmp32.mir
@@ -0,0 +1,801 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=riscv32 -run-pass=instruction-select -simplify-mir \
+# RUN: -verify-machineinstrs %s -o - | FileCheck %s
+---
+name:            cmp_ult_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_ult_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLTU:%[0-9]+]]:gpr = SLTU [[COPY]], [[COPY1]]
+    ; CHECK-NEXT: $x10 = COPY [[SLTU]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(ult), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_slt_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_slt_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLT:%[0-9]+]]:gpr = SLT [[COPY]], [[COPY1]]
+    ; CHECK-NEXT: $x10 = COPY [[SLT]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(slt), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_ugt_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_ugt_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLTU:%[0-9]+]]:gpr = SLTU [[COPY1]], [[COPY]]
+    ; CHECK-NEXT: $x10 = COPY [[SLTU]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(ugt), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_sgt_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_sgt_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLT:%[0-9]+]]:gpr = SLT [[COPY1]], [[COPY]]
+    ; CHECK-NEXT: $x10 = COPY [[SLT]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(sgt), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_eq_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_eq_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[XOR:%[0-9]+]]:gpr = XOR [[COPY]], [[COPY1]]
+    ; CHECK-NEXT: [[SLTIU:%[0-9]+]]:gpr = SLTIU [[XOR]], 1
+    ; CHECK-NEXT: $x10 = COPY [[SLTIU]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(eq), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_ne_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_ne_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[XOR:%[0-9]+]]:gpr = XOR [[COPY]], [[COPY1]]
+    ; CHECK-NEXT: [[SLTU:%[0-9]+]]:gpr = SLTU $x0, [[XOR]]
+    ; CHECK-NEXT: $x10 = COPY [[SLTU]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(ne), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_ule_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_ule_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLTU:%[0-9]+]]:gpr = SLTU [[COPY1]], [[COPY]]
+    ; CHECK-NEXT: [[XORI:%[0-9]+]]:gpr = XORI [[SLTU]], 1
+    ; CHECK-NEXT: $x10 = COPY [[XORI]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(ule), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_sle_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_sle_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLT:%[0-9]+]]:gpr = SLT [[COPY1]], [[COPY]]
+    ; CHECK-NEXT: [[XORI:%[0-9]+]]:gpr = XORI [[SLT]], 1
+    ; CHECK-NEXT: $x10 = COPY [[XORI]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(sle), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_uge_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_uge_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLTU:%[0-9]+]]:gpr = SLTU [[COPY]], [[COPY1]]
+    ; CHECK-NEXT: [[XORI:%[0-9]+]]:gpr = XORI [[SLTU]], 1
+    ; CHECK-NEXT: $x10 = COPY [[XORI]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(uge), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_sge_i32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_sge_i32
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLT:%[0-9]+]]:gpr = SLT [[COPY]], [[COPY1]]
+    ; CHECK-NEXT: [[XORI:%[0-9]+]]:gpr = XORI [[SLT]], 1
+    ; CHECK-NEXT: $x10 = COPY [[XORI]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(s32) = COPY $x10
+    %1:gprb(s32) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(sge), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_ult_p0
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_ult_p0
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLTU:%[0-9]+]]:gpr = SLTU [[COPY]], [[COPY1]]
+    ; CHECK-NEXT: $x10 = COPY [[SLTU]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(p0) = COPY $x10
+    %1:gprb(p0) = COPY $x11
+    %2:gprb(s32) = G_ICMP intpred(ult), %0, %1
+    $x10 = COPY %2(s32)
+    PseudoRET implicit $x10
+
+...
+---
+name:            cmp_slt_p0
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $x11
+
+    ; CHECK-LABEL: name: cmp_slt_p0
+    ; CHECK: liveins: $x10, $x11
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr = COPY $x11
+    ; CHECK-NEXT: [[SLT:%[0-9]+]]:gpr = SLT [[COPY]], [[COPY1]]
+    ; CHECK-NEXT: $x10 = COPY [[SLT]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:gprb(p0)...
[truncated]

@github-actions
Copy link

github-actions bot commented Sep 22, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff c1afed9f48c2d4ebb21ac1672948e82d23c69174 d186c5a4379cb20634d42856cb101b941b514a11 -- llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp llvm/utils/TableGen/InfoByHwMode.cpp
View the diff from clang-format here.
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
index 0cb74205071d..bce210a29cd9 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
@@ -123,15 +123,12 @@ RISCVInstructionSelector::selectShiftMask(MachineOperand &Root) const {
   return {{[=](MachineInstrBuilder &MIB) { MIB.add(Root); }}};
 }
 
-
 InstructionSelector::ComplexRendererFns
 RISCVInstructionSelector::selectAddrRegImm(MachineOperand &Root) const {
   // TODO: Need to get the immediate from a G_PTR_ADD. Should this be done in
   // the combiner?
-  return {{
-        [=](MachineInstrBuilder &MIB) { MIB.addReg(Root.getReg()); },
-        [=](MachineInstrBuilder &MIB) { MIB.addImm(0); }
-    }};
+  return {{[=](MachineInstrBuilder &MIB) { MIB.addReg(Root.getReg()); },
+           [=](MachineInstrBuilder &MIB) { MIB.addImm(0); }}};
 }
 
 // Tablegen doesn't allow us to write SRLIW/SRAIW/SLLIW patterns because the

@@ -35,6 +35,8 @@ ValueTypeByHwMode::ValueTypeByHwMode(Record *R, const CodeGenHwModes &CGH) {
assert(I.second && "Duplicate entry?");
(void)I;
}
if (R->isSubClassOf("PtrValueType"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like hoisting this TableGen change (as well as its related one in llvm/Target/Target.td) to a separate patch will be more ideal, as they're generic changes that might deserve its own test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Github PRs don't support stacked diffs, @topperc and I decided that it would make sense for me to submit a multi-commit PR. When I get the LGTM, I'll manually apply each commit to the main branch and close this without squashing and merging.
Do you agree that this makes sense? If you do, I'll add some tests to the commit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

submit a multi-commit PR.

yeah I'm fine with that. Stacked review has been discussed for a while in the community and I doubt there will be any resolution (either workaround or supports from GH) anytime soon

@michaelmaitland
Copy link
Contributor

Should the commits here be individual patches? I think you'll be forced to squash your commits at the end and it would be nice to have each individual commit in the log.

@nitinjohnraj
Copy link
Contributor Author

Should the commits here be individual patches? I think you'll be forced to squash your commits at the end and it would be nice to have each individual commit in the log.

@topperc and I discussed that since we don't have stacked diffs on Github PRs it makes sense to create a PR with multiple commits. I have to be careful not to squash and merge when the PR gets the LGTM, and instead manually apply these commits to main and close this PR.

@michaelmaitland
Copy link
Contributor

michaelmaitland commented Sep 26, 2023

Should the commits here be individual patches? I think you'll be forced to squash your commits at the end and it would be nice to have each individual commit in the log.

@topperc and I discussed that since we don't have stacked diffs on Github PRs it makes sense to create a PR with multiple commits. I have to be careful not to squash and merge when the PR gets the LGTM, and instead manually apply these commits to main and close this PR.

Sounds good to me. Thanks for noting that in the PR convo for reference.

@nitinjohnraj nitinjohnraj changed the title [TableGen][RISCV][GlobalISel] Select G_ICMP, G_SELECT, G_PTR_ADD [TableGen][RISCV][GlobalISel] Select G_ICMP, G_SELECT, G_PTR_ADD, G_STORE, G_LOAD, G_ZEXTLOAD Sep 26, 2023
const MachineInstr &MI,
int OpIdx) const {
const MachineInstr &MI,
int OpIdx) const {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This formatting change should not be in the load/store patch.

@@ -124,3 +128,9 @@ def : Pat<(XLenVT (setle (Ty GPR:$rs1), (Ty simm12Minus1Nonzero:$imm))),
def : Pat<(XLenVT (setle (Ty GPR:$rs1), (Ty GPR:$rs2))),
(XORI (SLT GPR:$rs2, GPR:$rs1), 1)>;
}

// Define pattern expansions for load/extload operations on pointers
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need patterns with i32 result type on RV64 too.

@nitinjohnraj nitinjohnraj deleted the isel branch October 6, 2023 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants