[TRI][RISCV] Add methods to get common register class of two registers

[wangpc-pp]

Here we add two methods getCommonMinimalPhysRegClass and a LLT
version getCommonMinimalPhysRegClassLLT, which return the most
sub register class of the right type that contains these two input
registers.

We don't overload the getMinimalPhysRegClass as there will be
ambiguities.

We use it to simplify some code in RISC-V target.

[llvmbot]

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

Author: Pengcheng Wang (wangpc-pp)

Changes

Here we add two methods getCommonMinimalPhysRegClass and a LLT
version getCommonMinimalPhysRegClassLLT, which return the most
sub register class of the right type that contains these two input
registers.

We don't overload the getMinimalPhysRegClass as there will be
ambiguities.

We use it to simplify some code in RISC-V target.

---

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

3 Files Affected:

• (modified) llvm/include/llvm/CodeGen/TargetRegisterInfo.h (+15)
• (modified) llvm/lib/CodeGen/TargetRegisterInfo.cpp (+37)
• (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+5-11)

diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index 292fa3c94969be..d9dfbbc5d66219 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -350,6 +350,13 @@ class TargetRegisterInfo : public MCRegisterInfo {
   const TargetRegisterClass *getMinimalPhysRegClass(MCRegister Reg,
                                                     MVT VT = MVT::Other) const;
 
+  /// Returns the common Register Class of two physical registers of the given
+  /// type, picking the most sub register class of the right type that contains
+  /// these two physregs.
+  const TargetRegisterClass *
+  getCommonMinimalPhysRegClass(MCRegister Reg1, MCRegister Reg2,
+                               MVT VT = MVT::Other) const;
+
   /// Returns the Register Class of a physical register of the given type,
   /// picking the most sub register class of the right type that contains this
   /// physreg. If there is no register class compatible with the given type,
@@ -357,6 +364,14 @@ class TargetRegisterInfo : public MCRegisterInfo {
   const TargetRegisterClass *getMinimalPhysRegClassLLT(MCRegister Reg,
                                                        LLT Ty = LLT()) const;
 
+  /// Returns the common Register Class of two physical registers of the given
+  /// type, picking the most sub register class of the right type that contains
+  /// these two physregs. If there is no register class compatible with the
+  /// given type, returns nullptr.
+  const TargetRegisterClass *
+  getCommonMinimalPhysRegClassLLT(MCRegister Reg1, MCRegister Reg2,
+                                  LLT Ty = LLT()) const;
+
   /// Return the maximal subclass of the given register class that is
   /// allocatable or NULL.
   const TargetRegisterClass *
diff --git a/llvm/lib/CodeGen/TargetRegisterInfo.cpp b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
index 032f1a33e75c43..4eff8bdbf744be 100644
--- a/llvm/lib/CodeGen/TargetRegisterInfo.cpp
+++ b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
@@ -222,6 +222,25 @@ TargetRegisterInfo::getMinimalPhysRegClass(MCRegister reg, MVT VT) const {
   return BestRC;
 }
 
+const TargetRegisterClass *TargetRegisterInfo::getCommonMinimalPhysRegClass(
+    MCRegister Reg1, MCRegister Reg2, MVT VT) const {
+  assert(Register::isPhysicalRegister(Reg1) &&
+         Register::isPhysicalRegister(Reg2) &&
+         "Reg1/Reg2 must be a physical register");
+
+  // Pick the most sub register class of the right type that contains
+  // this physreg.
+  const TargetRegisterClass *BestRC = nullptr;
+  for (const TargetRegisterClass *RC : regclasses()) {
+    if ((VT == MVT::Other || isTypeLegalForClass(*RC, VT)) &&
+        RC->contains(Reg1, Reg2) && (!BestRC || BestRC->hasSubClass(RC)))
+      BestRC = RC;
+  }
+
+  assert(BestRC && "Couldn't find the register class");
+  return BestRC;
+}
+
 const TargetRegisterClass *
 TargetRegisterInfo::getMinimalPhysRegClassLLT(MCRegister reg, LLT Ty) const {
   assert(Register::isPhysicalRegister(reg) &&
@@ -239,6 +258,24 @@ TargetRegisterInfo::getMinimalPhysRegClassLLT(MCRegister reg, LLT Ty) const {
   return BestRC;
 }
 
+const TargetRegisterClass *TargetRegisterInfo::getCommonMinimalPhysRegClassLLT(
+    MCRegister Reg1, MCRegister Reg2, LLT Ty) const {
+  assert(Register::isPhysicalRegister(Reg1) &&
+         Register::isPhysicalRegister(Reg2) &&
+         "Reg1/Reg2 must be a physical register");
+
+  // Pick the most sub register class of the right type that contains
+  // this physreg.
+  const TargetRegisterClass *BestRC = nullptr;
+  for (const TargetRegisterClass *RC : regclasses()) {
+    if ((!Ty.isValid() || isTypeLegalForClass(*RC, Ty)) &&
+        RC->contains(Reg1, Reg2) && (!BestRC || BestRC->hasSubClass(RC)))
+      BestRC = RC;
+  }
+
+  return BestRC;
+}
+
 /// getAllocatableSetForRC - Toggle the bits that represent allocatable
 /// registers for the specific register class.
 static void getAllocatableSetForRC(const MachineFunction &MF,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 47273d6bc06d65..017a1c514879b4 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -562,17 +562,11 @@ void RISCVInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
   }
 
   // VR->VR copies.
-  static const TargetRegisterClass *RVVRegClasses[] = {
-      &RISCV::VRRegClass,     &RISCV::VRM2RegClass,   &RISCV::VRM4RegClass,
-      &RISCV::VRM8RegClass,   &RISCV::VRN2M1RegClass, &RISCV::VRN2M2RegClass,
-      &RISCV::VRN2M4RegClass, &RISCV::VRN3M1RegClass, &RISCV::VRN3M2RegClass,
-      &RISCV::VRN4M1RegClass, &RISCV::VRN4M2RegClass, &RISCV::VRN5M1RegClass,
-      &RISCV::VRN6M1RegClass, &RISCV::VRN7M1RegClass, &RISCV::VRN8M1RegClass};
-  for (const auto &RegClass : RVVRegClasses) {
-    if (RegClass->contains(DstReg, SrcReg)) {
-      copyPhysRegVector(MBB, MBBI, DL, DstReg, SrcReg, KillSrc, RegClass);
-      return;
-    }
+  const TargetRegisterClass *RegClass =
+      TRI->getCommonMinimalPhysRegClass(SrcReg, DstReg);
+  if (RISCVRegisterInfo::isRVVRegClass(RegClass)) {
+    copyPhysRegVector(MBB, MBBI, DL, DstReg, SrcReg, KillSrc, RegClass);
+    return;
   }
 
   llvm_unreachable("Impossible reg-to-reg copy");

[ilovepi]

The two functions are almost identical, can we maybe use a template to contain all the inner logic? The only difference I see that's hard to factor out is Ty.isValid() vs VT == MVT::Other. Since they're both bools, maybe that could be passed in as a param, and the templating could take care of the rest?

Its very little code, but these kind of need to be kept in sync otherwise.

[wangpc-pp]

Is it possible to represent default argument value in a template function?

[ilovepi]

I was thinking more that you'd just put all the logic in a new function that was templated, and let each of these just call that internally. Then you can keep your API as is (assuming it needs to be that way for a reason) and the two won't get out of sync. The only thing the original functions need to do is evaluate the boolean, and then return the call to the common impl.

so Somthing like this, where the current functions are just think wrappers.

const TargetRegisterClass *TargetRegisterInfo::getCommonMinimalPhysRegClass(MCRegister Reg1, MCRegister Reg2, MVT VT) const {
  bool isValid = VT == MVT::Other;
  return commonImpl( Reg1, Reg2, VT, isValid);
}

[wangpc-pp]

Done. I used type traits to decide the value, please have a look.