[RISCV][GISEL] Add vector RegisterBanks and vector support in getRegBankFromRegClass #71541

michaelmaitland · 2023-11-07T14:30:06Z

Vector Register banks are created for the various register vector
register groupings. getRegBankFromRegClass is implemented to go from
vector TargetRegisterClass to the corresponding vector RegisterBank.

TypeSize is used in places needed to prevent RegBankSelectionFrom failing.

llvmbot · 2023-11-07T14:30:30Z

@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-llvm-regalloc
@llvm/pr-subscribers-llvm-globalisel

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

Author: Michael Maitland (michaelmaitland)

Changes

Vector Register banks are created for the various register vector
register groupings. getRegBankFromRegClass is implemented to go from
vector TargetRegisterClass to the corresponding vector RegisterBank.

TypeSize is used in places needed to prevent RegBankSelectionFrom failing.

This change is stacked on #70881

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

10 Files Affected:

(modified) llvm/include/llvm/CodeGen/RegisterBankInfo.h (+2-2)
(modified) llvm/include/llvm/CodeGen/TargetRegisterInfo.h (+3-3)
(modified) llvm/lib/CodeGen/MachineVerifier.cpp (+12-10)
(modified) llvm/lib/CodeGen/RegisterBankInfo.cpp (+5-4)
(modified) llvm/lib/CodeGen/TargetRegisterInfo.cpp (+9-10)
(modified) llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp (+20)
(modified) llvm/lib/Target/RISCV/GISel/RISCVRegisterBanks.td (+12)
(modified) llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll (+2-2)
(added) llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/vec-args.mir (+758)
(added) llvm/test/MachineVerifier/copy-scalable.mir (+23)

diff --git a/llvm/include/llvm/CodeGen/RegisterBankInfo.h b/llvm/include/llvm/CodeGen/RegisterBankInfo.h
index 1ee1f6b6c32ed63..b353ea8b3cc86ec 100644
--- a/llvm/include/llvm/CodeGen/RegisterBankInfo.h
+++ b/llvm/include/llvm/CodeGen/RegisterBankInfo.h
@@ -177,7 +177,7 @@ class RegisterBankInfo {
     /// \note This method does not check anything when assertions are disabled.
     ///
     /// \return True is the check was successful.
-    bool verify(const RegisterBankInfo &RBI, unsigned MeaningfulBitWidth) const;
+    bool verify(const RegisterBankInfo &RBI, TypeSize MeaningfulBitWidth) const;
 
     /// Print this on dbgs() stream.
     void dump() const;
@@ -749,7 +749,7 @@ class RegisterBankInfo {
   /// virtual register.
   ///
   /// \pre \p Reg != 0 (NoRegister).
-  unsigned getSizeInBits(Register Reg, const MachineRegisterInfo &MRI,
+  TypeSize getSizeInBits(Register Reg, const MachineRegisterInfo &MRI,
                          const TargetRegisterInfo &TRI) const;
 
   /// Check that information hold by this instance make sense for the
diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index 337fab735a09522..4fb6ba7c26930af 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -283,8 +283,8 @@ class TargetRegisterInfo : public MCRegisterInfo {
   // DenseMapInfo<unsigned> uses -1u and -2u.
 
   /// Return the size in bits of a register from class RC.
-  unsigned getRegSizeInBits(const TargetRegisterClass &RC) const {
-    return getRegClassInfo(RC).RegSize;
+  TypeSize getRegSizeInBits(const TargetRegisterClass &RC) const {
+    return TypeSize::Fixed(getRegClassInfo(RC).RegSize);
   }
 
   /// Return the size in bytes of the stack slot allocated to hold a spilled
@@ -858,7 +858,7 @@ class TargetRegisterInfo : public MCRegisterInfo {
     const TargetRegisterClass *RC) const = 0;
 
   /// Returns size in bits of a phys/virtual/generic register.
-  unsigned getRegSizeInBits(Register Reg, const MachineRegisterInfo &MRI) const;
+  TypeSize getRegSizeInBits(Register Reg, const MachineRegisterInfo &MRI) const;
 
   /// Get the weight in units of pressure for this register unit.
   virtual unsigned getRegUnitWeight(unsigned RegUnit) const = 0;
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index dadaf60fa09da04..8f2c42bfac88229 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1937,8 +1937,8 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
 
     // If we have only one valid type, this is likely a copy between a virtual
     // and physical register.
-    unsigned SrcSize = 0;
-    unsigned DstSize = 0;
+    TypeSize SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
+    TypeSize DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
     if (SrcReg.isPhysical() && DstTy.isValid()) {
       const TargetRegisterClass *SrcRC =
           TRI->getMinimalPhysRegClassLLT(SrcReg, DstTy);
@@ -1946,9 +1946,6 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
         SrcSize = TRI->getRegSizeInBits(*SrcRC);
     }
 
-    if (SrcSize == 0)
-      SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
-
     if (DstReg.isPhysical() && SrcTy.isValid()) {
       const TargetRegisterClass *DstRC =
           TRI->getMinimalPhysRegClassLLT(DstReg, SrcTy);
@@ -1956,10 +1953,15 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
         DstSize = TRI->getRegSizeInBits(*DstRC);
     }
 
-    if (DstSize == 0)
-      DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
+    // If this is a copy from physical register to virtual register, and if the
+    // Dst is scalable and the Src is fixed, then the Dst can only hold the Src
+    // if the minimum size Dst can hold is at least as big as Src.
+    if (SrcReg.isPhysical() && DstReg.isVirtual() && DstSize.isScalable() &&
+        !SrcSize.isScalable() &&
+        DstSize.getKnownMinValue() <= SrcSize.getFixedValue())
+      break;
 
-    if (SrcSize != 0 && DstSize != 0 && SrcSize != DstSize) {
+    if (SrcSize.isNonZero() && DstSize.isNonZero() && SrcSize != DstSize) {
       if (!DstOp.getSubReg() && !SrcOp.getSubReg()) {
         report("Copy Instruction is illegal with mismatching sizes", MI);
         errs() << "Def Size = " << DstSize << ", Src Size = " << SrcSize
@@ -2254,8 +2256,8 @@ MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
           }
 
           // Make sure the register fits into its register bank if any.
-          if (RegBank && Ty.isValid() &&
-              RBI->getMaximumSize(RegBank->getID()) < Ty.getSizeInBits()) {
+          if (RegBank && Ty.isValid() && (!Ty.isScalable() &&
+              RBI->getMaximumSize(RegBank->getID()) < Ty.getSizeInBits())) {
             report("Register bank is too small for virtual register", MO,
                    MONum);
             errs() << "Register bank " << RegBank->getName() << " too small("
diff --git a/llvm/lib/CodeGen/RegisterBankInfo.cpp b/llvm/lib/CodeGen/RegisterBankInfo.cpp
index f9721d7d9386958..6a96bb40f56aed9 100644
--- a/llvm/lib/CodeGen/RegisterBankInfo.cpp
+++ b/llvm/lib/CodeGen/RegisterBankInfo.cpp
@@ -495,7 +495,7 @@ void RegisterBankInfo::applyDefaultMapping(const OperandsMapper &OpdMapper) {
   }
 }
 
-unsigned RegisterBankInfo::getSizeInBits(Register Reg,
+TypeSize RegisterBankInfo::getSizeInBits(Register Reg,
                                          const MachineRegisterInfo &MRI,
                                          const TargetRegisterInfo &TRI) const {
   if (Reg.isPhysical()) {
@@ -553,7 +553,7 @@ bool RegisterBankInfo::ValueMapping::partsAllUniform() const {
 }
 
 bool RegisterBankInfo::ValueMapping::verify(const RegisterBankInfo &RBI,
-                                            unsigned MeaningfulBitWidth) const {
+                                            TypeSize MeaningfulBitWidth) const {
   assert(NumBreakDowns && "Value mapped nowhere?!");
   unsigned OrigValueBitWidth = 0;
   for (const RegisterBankInfo::PartialMapping &PartMap : *this) {
@@ -565,8 +565,9 @@ bool RegisterBankInfo::ValueMapping::verify(const RegisterBankInfo &RBI,
     OrigValueBitWidth =
         std::max(OrigValueBitWidth, PartMap.getHighBitIdx() + 1);
   }
-  assert(OrigValueBitWidth >= MeaningfulBitWidth &&
-         "Meaningful bits not covered by the mapping");
+  assert(MeaningfulBitWidth.isScalable() ||
+         OrigValueBitWidth >= MeaningfulBitWidth &&
+             "Meaningful bits not covered by the mapping");
   APInt ValueMask(OrigValueBitWidth, 0);
   for (const RegisterBankInfo::PartialMapping &PartMap : *this) {
     // Check that the union of the partial mappings covers the whole value,
diff --git a/llvm/lib/CodeGen/TargetRegisterInfo.cpp b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
index 1bb35f40facfd0f..c50b1cf9422717a 100644
--- a/llvm/lib/CodeGen/TargetRegisterInfo.cpp
+++ b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
@@ -499,7 +499,7 @@ bool TargetRegisterInfo::regmaskSubsetEqual(const uint32_t *mask0,
   return true;
 }
 
-unsigned
+TypeSize
 TargetRegisterInfo::getRegSizeInBits(Register Reg,
                                      const MachineRegisterInfo &MRI) const {
   const TargetRegisterClass *RC{};
@@ -508,16 +508,15 @@ TargetRegisterInfo::getRegSizeInBits(Register Reg,
     // Instead, we need to access a register class that contains Reg and
     // get the size of that register class.
     RC = getMinimalPhysRegClass(Reg);
-  } else {
-    LLT Ty = MRI.getType(Reg);
-    unsigned RegSize = Ty.isValid() ? Ty.getSizeInBits() : 0;
-    // If Reg is not a generic register, query the register class to
-    // get its size.
-    if (RegSize)
-      return RegSize;
-    // Since Reg is not a generic register, it must have a register class.
-    RC = MRI.getRegClass(Reg);
+    assert(RC && "Unable to deduce the register class");
+    return getRegSizeInBits(*RC);
   }
+  LLT Ty = MRI.getType(Reg);
+  if (Ty.isValid())
+    return Ty.getSizeInBits();
+
+  // Since Reg is not a generic register, it may have a register class.
+  RC = MRI.getRegClass(Reg);
   assert(RC && "Unable to deduce the register class");
   return getRegSizeInBits(*RC);
 }
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
index 3fc00cb760358bb..7d2c151752549c3 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
@@ -100,6 +100,26 @@ RISCVRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
   case RISCV::FPR64CRegClassID:
   case RISCV::FPR32CRegClassID:
     return getRegBank(RISCV::FPRRegBankID);
+  case RISCV::VRRegClassID:
+    return getRegBank(RISCV::VRRegBankID);
+  case RISCV::VRNoV0RegClassID:
+    return getRegBank(RISCV::VRNoV0RegBankID);
+  case RISCV::VRM2RegClassID:
+    return getRegBank(RISCV::VRM2RegBankID);
+  case RISCV::VRM2NoV0RegClassID:
+    return getRegBank(RISCV::VRM2NoV0RegBankID);
+  case RISCV::VRM4RegClassID:
+    return getRegBank(RISCV::VRM4RegBankID);
+  case RISCV::VRM4NoV0RegClassID:
+    return getRegBank(RISCV::VRM4NoV0RegBankID);
+  case RISCV::VRM8RegClassID:
+    return getRegBank(RISCV::VRM8RegBankID);
+  case RISCV::VRM8NoV0RegClassID:
+    return getRegBank(RISCV::VRM8NoV0RegBankID);
+  case RISCV::VMRegClassID:
+    return getRegBank(RISCV::VMRegBankID);
+  case RISCV::VMV0RegClassID:
+    return getRegBank(RISCV::VMV0RegBankID);
   }
 }
 
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBanks.td b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBanks.td
index 49f18e19c2269fd..40ef98d2badde1d 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBanks.td
+++ b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBanks.td
@@ -14,3 +14,15 @@ def GPRRegBank : RegisterBank<"GPRB", [GPR]>;
 
 /// Floating Point Registers: F.
 def FPRRegBank : RegisterBank<"FPRB", [FPR64]>;
+
+/// Vector Register Banks:
+def VRRegBank : RegisterBank<"VRB", [VR]>;
+def VRNoV0RegBank : RegisterBank<"VRNoV0B", [VRNoV0]>;
+def VRM2RegBank : RegisterBank<"VRM2B", [VRM2]>;
+def VRM2NoV0RegBank : RegisterBank<"VRM2NoV0B", [VRM2NoV0]>;
+def VRM4RegBank : RegisterBank<"VRM4B", [VRM4]>;
+def VRM4NoV0RegBank : RegisterBank<"VRM4NoV0B", [VRM4NoV0]>;
+def VRM8RegBank : RegisterBank<"VRM8B", [VRM8]>;
+def VRM8NoV0RegBank : RegisterBank<"VRM8NoV0B", [VRM8NoV0]>;
+def VMRegBank : RegisterBank<"VMB", [VM]>;
+def VMV0RegBank : RegisterBank<"VMNoV0B", [VMV0]>;
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll b/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll
index 5dd62de8a6bc415..a3a913d8ce02d83 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll
@@ -22,7 +22,7 @@ entry:
   ret <vscale x 1 x i8> %a
 }
 
-; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to translate instruction{{.*}}scalable_inst
+; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to translate instruction: call:
 ; FALLBACK-WITH-REPORT-OUT-LABEL: scalable_inst
 define <vscale x 1 x i8> @scalable_inst(i64 %0) nounwind {
 entry:
@@ -35,7 +35,7 @@ entry:
   ret <vscale x 1 x i8> %a
 }
 
-; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to translate instruction{{.*}}scalable_alloca
+; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to translate instruction: alloca:
 ; FALLBACK-WITH-REPORT-OUT-LABEL: scalable_alloca
 define void @scalable_alloca() #1 {
   %local0 = alloca <vscale x 16 x i8>
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/vec-args.mir b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/vec-args.mir
new file mode 100644
index 000000000000000..9574f4203a0d5e7
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/vec-args.mir
@@ -0,0 +1,758 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32  -run-pass=regbankselect \
+# RUN:   -disable-gisel-legality-check -simplify-mir -verify-machineinstrs %s \
+# RUN:   -o - | FileCheck -check-prefixes=BOTH,RV32 %s
+# RUN: llc -mtriple=riscv64  -run-pass=regbankselect \
+# RUN:   -disable-gisel-legality-check -simplify-mir -verify-machineinstrs %s \
+# RUN:   -o - | FileCheck -check-prefixes=BOTH,RV64 %s
+
+---
+name:            test_args_nxv1i8
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv1i8
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 1 x s8>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 1 x s8>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv2i8
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv2i8
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 2 x s8>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 2 x s8>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv4i8
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv4i8
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 4 x s8>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 4 x s8>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv8i8
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv8i8
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 8 x s8>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 8 x s8>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv16i8
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m2
+    ; BOTH-LABEL: name: test_args_nxv16i8
+    ; BOTH: liveins: $v8m2
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm2nov0b(<vscale x 16 x s8>) = COPY $v8m2
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 16 x s8>) = COPY $v8m2
+    PseudoRET
+...
+---
+name:            test_args_nxv32i8
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m4
+    ; BOTH-LABEL: name: test_args_nxv32i8
+    ; BOTH: liveins: $v8m4
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm4nov0b(<vscale x 32 x s8>) = COPY $v8m4
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 32 x s8>) = COPY $v8m4
+    PseudoRET
+...
+---
+name:            test_args_nxv64i8
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m8
+    ; BOTH-LABEL: name: test_args_nxv64i8
+    ; BOTH: liveins: $v8m8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm8nov0b(<vscale x 64 x s8>) = COPY $v8m8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 64 x s8>) = COPY $v8m8
+    PseudoRET
+...
+---
+name:            test_args_nxv1i16
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv1i16
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 1 x s16>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 1 x s16>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv2i16
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv2i16
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 2 x s16>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 2 x s16>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv4i16
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv4i16
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 4 x s16>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 4 x s16>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv8i16
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m2
+    ; BOTH-LABEL: name: test_args_nxv8i16
+    ; BOTH: liveins: $v8m2
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm2nov0b(<vscale x 8 x s16>) = COPY $v8m2
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 8 x s16>) = COPY $v8m2
+    PseudoRET
+...
+---
+name:            test_args_nxv16i16
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m4
+    ; BOTH-LABEL: name: test_args_nxv16i16
+    ; BOTH: liveins: $v8m4
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm4nov0b(<vscale x 16 x s16>) = COPY $v8m4
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 16 x s16>) = COPY $v8m4
+    PseudoRET
+...
+---
+name:            test_args_nxv32i16
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m8
+    ; BOTH-LABEL: name: test_args_nxv32i16
+    ; BOTH: liveins: $v8m8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm8nov0b(<vscale x 32 x s16>) = COPY $v8m8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 32 x s16>) = COPY $v8m8
+    PseudoRET
+...
+---
+name:            test_args_nxv1i32
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv1i32
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 1 x s32>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 1 x s32>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv2i32
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv2i32
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 2 x s32>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 2 x s32>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv4i32
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m2
+    ; BOTH-LABEL: name: test_args_nxv4i32
+    ; BOTH: liveins: $v8m2
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm2nov0b(<vscale x 4 x s32>) = COPY $v8m2
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 4 x s32>) = COPY $v8m2
+    PseudoRET
+...
+---
+name:            test_args_nxv8i32
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m4
+    ; BOTH-LABEL: name: test_args_nxv8i32
+    ; BOTH: liveins: $v8m4
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm4nov0b(<vscale x 8 x s32>) = COPY $v8m4
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 8 x s32>) = COPY $v8m4
+    PseudoRET
+...
+---
+name:            test_args_nxv16i32
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m8
+    ; BOTH-LABEL: name: test_args_nxv16i32
+    ; BOTH: liveins: $v8m8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm8nov0b(<vscale x 16 x s32>) = COPY $v8m8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 16 x s32>) = COPY $v8m8
+    PseudoRET
+...
+---
+name:            test_args_nxv1i64
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8
+    ; BOTH-LABEL: name: test_args_nxv1i64
+    ; BOTH: liveins: $v8
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrnov0b(<vscale x 1 x s64>) = COPY $v8
+    ; BOTH-NEXT: PseudoRET
+    %0:_(<vscale x 1 x s64>) = COPY $v8
+    PseudoRET
+...
+---
+name:            test_args_nxv2i64
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $v8m2
+    ; BOTH-LABEL: name: test_args_nxv2i64
+    ; BOTH: liveins: $v8m2
+    ; BOTH-NEXT: {{  $}}
+    ; BOTH-NEXT: [[COPY:%[0-9]+]]:vrm2nov0b(<vscale x 2 x s64>) = COPY $v8m2
+    ; BOTH-NEXT: PseudoRET...
[truncated]

github-actions · 2023-11-07T14:39:04Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvm/lib/CodeGen/MachineVerifier.cpp

topperc · 2023-11-09T19:38:05Z

llvm/lib/Target/RISCV/GISel/RISCVRegisterBanks.td

@@ -14,3 +14,15 @@ def GPRRegBank : RegisterBank<"GPRB", [GPR]>;

 /// Floating Point Registers: F.
 def FPRRegBank : RegisterBank<"FPRB", [FPR64]>;
+
+/// Vector Register Banks:
+def VRRegBank : RegisterBank<"VRB", [VR]>;


There should only be one vector register bank. The register bank should be the collection of all vector registers. It's not one bank for each register class.

From the regbankinfo api and gmir-regbank docs:

* Banks: addRegBankCoverage — which register bank covers each register class. * Register Banks are a means to constrain the register allocator to use a particular register file for a virtual register.

The current approach in this patch creates reg bank covers which try to be as small as they can be. For example, if we know that it has a VRM2 class, we choose a VRM2 bank instead of the more broad VR bank. My aim was that we'd constrain the register allocator to only use register that work for M2.

The approach you suggest creates reg bank covers that include the entire register file (the vector registers). What does this achieve if we cannot use every register in the register file anyway?

On a separate, but related note, do you think should VR and VM registers have different banks?

Note that description for Register Banks says "particular register file". The vector registers make up a single register file thus should be a single register bank.

My aim was that we'd constrain the register allocator to only use register that work for M2.

The register allocator doesn't know about register banks. It only knows about register classes. RISCVInstructionSelector::getRegClassForTypeOnBank is responsible for converting from RegBank+LLT to register class. LMUL is encoded in the scalable vector type so you should be able to pick the correct LMUL register class from a single bank.

On a separate, but related note, do you think should VR and VM registers have different banks?

Single register bank.

RISCVInstructionSelector::getRegClassForTypeOnBank is responsible for converting from RegBank+LLT to register class.

It looks like its going from RegClass + LLT -> RegBank. I guess I am confused what the register banks are useful for?

The register allocator doesn't know about register banks.

Are you sure this is the case? The docs say Register Banks are a means to constrain the register allocator....

It looks like its going from RegClass + LLT -> RegBank. I guess I am confused what the register banks are useful for?

It should be RegBank+LLT->RegClass like the function name says. It returns 3 values today, RISCV::GPRRegClass, RISCV::FPR32RegClass or RISCV::FPR64RegClass

Are you sure this is the case? The docs say Register Banks are a means to constrain the register allocator....

Once we do instruction selection every register operand or a selected instruction is a virtual register with a register class assigned to it. The register bank and LLT are gone, or at least not used later.

Register banks are specifically vaguer than register classes. They are supposed to express how expensive and/or impossible it is to copy between registers of a class. e.g. it might be expensive to copy values between GPRs and FPRs, but the bank doesn't need to concern itself with the specific operand constraints of individual instructions

topperc · 2023-11-10T23:20:40Z

llvm/lib/Target/RISCV/GISel/RISCVRegisterBanks.td

@@ -14,3 +14,7 @@ def GPRRegBank : RegisterBank<"GPRB", [GPR]>;

 /// Floating Point Registers: F.
 def FPRRegBank : RegisterBank<"FPRB", [FPR64]>;
+
+/// Vector Regististers: V.
+def VRRegBank : RegisterBank<"VRB", [VR]>;


Is [VR] enough to make tablegen consider all of the register classes as being part of the bank? You can check theVRRegBankCoverageData in lib/Target/RISCV/RISCVGenRegisterBank.inc in your build directory.

I suspect you need use [VRM8]

llvm/lib/Target/RISCV/GISel/RISCVRegisterBanks.td

arsenm · 2023-11-14T01:45:11Z

llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h

@@ -165,7 +165,7 @@ class AMDGPURegisterBankInfo final : public AMDGPUGenRegisterBankInfo {
  bool isDivergentRegBank(const RegisterBank *RB) const override;

  unsigned copyCost(const RegisterBank &A, const RegisterBank &B,
-                    unsigned Size) const override;


Can you pre-commit all of these unsigned->TypeSize pieces?

I have them separated into their own commit. It is the first commit in the commit history of this Pr. I will commit it separate.

This is a precommit for llvm#71514 to use TypeSize instead of unsigned to avoid crashes when scalable vectors are used.

Scalable vector types from LLVM IR are lowered into physical vector registers in MIR based on calling convention. This patch is stacked on llvm#70881.

…ankFromRegClass Vector Register banks are created for the various register vector register groupings. getRegBankFromRegClass is implemented to go from vector TargetRegisterClass to the corresponding vector RegisterBank.

topperc

LGTM

michaelmaitland · 2023-11-16T15:57:17Z

This PR has been committed in f219e03 and dbd884c.

michaelmaitland added backend:RISC-V llvm:globalisel labels Nov 7, 2023

michaelmaitland requested review from arsenm, mshockwave, tschuett and topperc November 7, 2023 14:30

llvmbot added the llvm:regalloc label Nov 7, 2023

michaelmaitland force-pushed the vec-regbank-main branch 2 times, most recently from db2a757 to 9a5ffdf Compare November 7, 2023 20:29

arsenm reviewed Nov 8, 2023

View reviewed changes

llvm/lib/CodeGen/MachineVerifier.cpp Outdated Show resolved Hide resolved

michaelmaitland force-pushed the vec-regbank-main branch from 9a5ffdf to f7f87e4 Compare November 8, 2023 18:24

llvmbot added backend:AArch64 backend:AMDGPU labels Nov 8, 2023

michaelmaitland force-pushed the vec-regbank-main branch from c2a7df1 to ac9e36f Compare November 9, 2023 19:34

topperc reviewed Nov 9, 2023

View reviewed changes

topperc reviewed Nov 10, 2023

View reviewed changes

topperc reviewed Nov 13, 2023

View reviewed changes

llvm/lib/Target/RISCV/GISel/RISCVRegisterBanks.td Outdated Show resolved Hide resolved

michaelmaitland force-pushed the vec-regbank-main branch from ac738e6 to c62bbbd Compare November 13, 2023 20:55

arsenm reviewed Nov 14, 2023

View reviewed changes

michaelmaitland force-pushed the vec-regbank-main branch 2 times, most recently from 3357512 to e6f5f97 Compare November 14, 2023 21:20

michaelmaitland added 2 commits November 14, 2023 13:34

[RISCV] Use TypeSize in places where needed for RegBankSelection

a1a77bb

This is a precommit for llvm#71514 to use TypeSize instead of unsigned to avoid crashes when scalable vectors are used.

[RISCV][GISEL] Add support for scalable vector types in lowerReturnVal

16a10c3

Scalable vector types from LLVM IR are lowered into physical vector registers in MIR based on calling convention. This patch is stacked on llvm#70881.

michaelmaitland force-pushed the vec-regbank-main branch from e6f5f97 to 1569d6f Compare November 14, 2023 21:35

michaelmaitland added 4 commits November 14, 2023 13:38

single vector regbank

84f05cb

update vrbregbank to use vrm8

f7a4d6f

add more vector reg classes

bafe162

michaelmaitland force-pushed the vec-regbank-main branch from 1569d6f to bafe162 Compare November 14, 2023 21:38

topperc approved these changes Nov 15, 2023

View reviewed changes

michaelmaitland closed this Nov 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RISCV][GISEL] Add vector RegisterBanks and vector support in getRegBankFromRegClass #71541

[RISCV][GISEL] Add vector RegisterBanks and vector support in getRegBankFromRegClass #71541

michaelmaitland commented Nov 7, 2023 •

edited

llvmbot commented Nov 7, 2023 •

edited

github-actions bot commented Nov 7, 2023 •

edited

topperc Nov 9, 2023 •

edited

michaelmaitland Nov 9, 2023

topperc Nov 9, 2023

michaelmaitland Nov 9, 2023

topperc Nov 9, 2023

topperc Nov 9, 2023

arsenm Nov 14, 2023

topperc Nov 10, 2023 •

edited

arsenm Nov 14, 2023

michaelmaitland Nov 14, 2023

topperc left a comment

michaelmaitland commented Nov 16, 2023

[RISCV][GISEL] Add vector RegisterBanks and vector support in getRegBankFromRegClass #71541

[RISCV][GISEL] Add vector RegisterBanks and vector support in getRegBankFromRegClass #71541

Conversation

michaelmaitland commented Nov 7, 2023 • edited

llvmbot commented Nov 7, 2023 • edited

github-actions bot commented Nov 7, 2023 • edited

topperc Nov 9, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topperc Nov 10, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topperc left a comment

Choose a reason for hiding this comment

michaelmaitland commented Nov 16, 2023

michaelmaitland commented Nov 7, 2023 •

edited

llvmbot commented Nov 7, 2023 •

edited

github-actions bot commented Nov 7, 2023 •

edited

topperc Nov 9, 2023 •

edited

topperc Nov 10, 2023 •

edited