[AArch64][GlobalISel] TableGen Selection for G_VECREDUCE_ADD

[chuongg3]

Instruction Selection for G_VECREDUCE_ADD now uses TableGen

[llvmbot]

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-aarch64

Author: None (chuongg3)

Changes

Instruction Selection for G_VECREDUCE_ADD now uses TableGen

---

Full diff: https://github.com/llvm/llvm-project/pull/70785.diff

3 Files Affected:

• (modified) llvm/lib/Target/AArch64/AArch64InstrGISel.td (+2)
• (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.td (+15)
• (modified) llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp (-45)

diff --git a/llvm/lib/Target/AArch64/AArch64InstrGISel.td b/llvm/lib/Target/AArch64/AArch64InstrGISel.td
index 27338bd24393325..4124b66ab1bf3e7 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrGISel.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrGISel.td
@@ -270,6 +270,8 @@ def : GINodeEquiv<G_BSP, AArch64bsp>;
 def : GINodeEquiv<G_UMULL, AArch64umull>;
 def : GINodeEquiv<G_SMULL, AArch64smull>;
 
+def : GINodeEquiv<G_VECREDUCE_ADD, vecreduce_add>;
+
 def : GINodeEquiv<G_EXTRACT_VECTOR_ELT, vector_extract>;
 
 def : GINodeEquiv<G_PREFETCH, AArch64Prefetch>;
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index ee42612c0fcdd2a..77edb94f0af3fe3 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -6637,6 +6637,21 @@ def : Pat<(i32 (and (i32 (vector_extract (opNode (v8i16 V128:$Rn)), (i64 0))),
           ssub))>;
 }
 
+def : Pat<(i8 (vecreduce_add (v8i8 V64:$Rn))), 
+          (i8 (ADDVv8i8v V64:$Rn))>;
+def : Pat<(i8 (vecreduce_add (v16i8 V128:$Rn))), 
+          (i8 (ADDVv16i8v V128:$Rn))>;
+def : Pat<(i16 (vecreduce_add (v4i16 V64:$Rn))), 
+          (i16 (ADDVv4i16v V64:$Rn))>;
+def : Pat<(i16 (vecreduce_add (v8i16 V128:$Rn))), 
+          (i16 (ADDVv8i16v V128:$Rn))>;
+def : Pat<(i32 (vecreduce_add (v2i32 V64:$Rn))), 
+          (i32 (EXTRACT_SUBREG (ADDPv2i32 V64:$Rn, V64:$Rn), ssub))>;
+def : Pat<(i32 (vecreduce_add (v4i32 V128:$Rn))), 
+          (i32 (ADDVv4i32v V128:$Rn))>;
+def : Pat<(i64 (vecreduce_add (v2i64 V128:$Rn))), 
+          (i64 (ADDPv2i64p V128:$Rn))>;
+
 defm : SIMDAcrossLanesSignedIntrinsic<"ADDV",  AArch64saddv>;
 // vaddv_[su]32 is special; -> ADDP Vd.2S,Vn.2S,Vm.2S; return Vd.s[0];Vn==Vm
 def : Pat<(v2i32 (AArch64saddv (v2i32 V64:$Rn))),
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 2089bfba5ff37c6..03e4b3b1bcbe90f 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -3557,8 +3557,6 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
     return selectConcatVectors(I, MRI);
   case TargetOpcode::G_JUMP_TABLE:
     return selectJumpTable(I, MRI);
-  case TargetOpcode::G_VECREDUCE_ADD:
-    return selectReduction(I, MRI);
   case TargetOpcode::G_MEMCPY:
   case TargetOpcode::G_MEMCPY_INLINE:
   case TargetOpcode::G_MEMMOVE:
@@ -3577,49 +3575,6 @@ bool AArch64InstructionSelector::selectAndRestoreState(MachineInstr &I) {
   return Success;
 }
 
-bool AArch64InstructionSelector::selectReduction(MachineInstr &I,
-                                                 MachineRegisterInfo &MRI) {
-  Register VecReg = I.getOperand(1).getReg();
-  LLT VecTy = MRI.getType(VecReg);
-  if (I.getOpcode() == TargetOpcode::G_VECREDUCE_ADD) {
-    // For <2 x i32> ADDPv2i32 generates an FPR64 value, so we need to emit
-    // a subregister copy afterwards.
-    if (VecTy == LLT::fixed_vector(2, 32)) {
-      Register DstReg = I.getOperand(0).getReg();
-      auto AddP = MIB.buildInstr(AArch64::ADDPv2i32, {&AArch64::FPR64RegClass},
-                                 {VecReg, VecReg});
-      auto Copy = MIB.buildInstr(TargetOpcode::COPY, {DstReg}, {})
-                      .addReg(AddP.getReg(0), 0, AArch64::ssub)
-                      .getReg(0);
-      RBI.constrainGenericRegister(Copy, AArch64::FPR32RegClass, MRI);
-      I.eraseFromParent();
-      return constrainSelectedInstRegOperands(*AddP, TII, TRI, RBI);
-    }
-
-    unsigned Opc = 0;
-    if (VecTy == LLT::fixed_vector(16, 8))
-      Opc = AArch64::ADDVv16i8v;
-    else if (VecTy == LLT::fixed_vector(8, 8))
-      Opc = AArch64::ADDVv8i8v;
-    else if (VecTy == LLT::fixed_vector(8, 16))
-      Opc = AArch64::ADDVv8i16v;
-    else if (VecTy == LLT::fixed_vector(4, 16))
-      Opc = AArch64::ADDVv4i16v;
-    else if (VecTy == LLT::fixed_vector(4, 32))
-      Opc = AArch64::ADDVv4i32v;
-    else if (VecTy == LLT::fixed_vector(2, 64))
-      Opc = AArch64::ADDPv2i64p;
-    else {
-      LLVM_DEBUG(dbgs() << "Unhandled type for add reduction");
-      return false;
-    }
-    I.setDesc(TII.get(Opc));
-    return constrainSelectedInstRegOperands(I, TII, TRI, RBI);
-  }
-
-  return false;
-}
-
 bool AArch64InstructionSelector::selectMOPS(MachineInstr &GI,
                                             MachineRegisterInfo &MRI) {
   unsigned Mopcode;

[dzhidzhoev]

Shouldn't this be located in llvm/lib/Target/AArch64/AArch64InstrGISel.td?

[davemgreen]

I think it's better to not have them separated out. It would be ideal to not have them at all, and have both SDAG and GISel share the same patterns, but sometimes it's useful to use the more natural type for the operations in the relevant ISel.

[davemgreen]

I think this looks OK, from what I can tell. LGTM

[davemgreen]

I think it's better to not have them separated out. It would be ideal to not have them at all, and have both SDAG and GISel share the same patterns, but sometimes it's useful to use the more natural type for the operations in the relevant ISel.