[VirtRegMap] Store MCRegister in Virt2PhysMap.

[topperc]

Remove NO_PHYS_REG in favor of MCRegister() and converting MCRegister to bool.

[llvmbot]

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-llvm-regalloc

@llvm/pr-subscribers-backend-amdgpu

Author: Craig Topper (topperc)

Changes

Remove NO_PHYS_REG in favor of MCRegister() and converting MCRegister to bool.

---

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

6 Files Affected:

• (modified) llvm/include/llvm/CodeGen/Register.h (-1)
• (modified) llvm/include/llvm/CodeGen/VirtRegMap.h (+7-13)
• (modified) llvm/include/llvm/MC/MCRegister.h (-1)
• (modified) llvm/lib/CodeGen/VirtRegMap.cpp (+5-5)
• (modified) llvm/lib/Target/AMDGPU/SILowerWWMCopies.cpp (+2-3)
• (modified) llvm/lib/Target/X86/X86TileConfig.cpp (+3-2)

diff --git a/llvm/include/llvm/CodeGen/Register.h b/llvm/include/llvm/CodeGen/Register.h
index fb4e5512111327..0ad576fec7d8de 100644
--- a/llvm/include/llvm/CodeGen/Register.h
+++ b/llvm/include/llvm/CodeGen/Register.h
@@ -132,7 +132,6 @@ class Register {
   /// Comparisons against register constants. E.g.
   /// * R == AArch64::WZR
   /// * R == 0
-  /// * R == VirtRegMap::NO_PHYS_REG
   constexpr bool operator==(unsigned Other) const { return Reg == Other; }
   constexpr bool operator!=(unsigned Other) const { return Reg != Other; }
   constexpr bool operator==(int Other) const { return Reg == unsigned(Other); }
diff --git a/llvm/include/llvm/CodeGen/VirtRegMap.h b/llvm/include/llvm/CodeGen/VirtRegMap.h
index fd99862c24bfac..96c28c191e8bd5 100644
--- a/llvm/include/llvm/CodeGen/VirtRegMap.h
+++ b/llvm/include/llvm/CodeGen/VirtRegMap.h
@@ -33,7 +33,6 @@ class TargetInstrInfo;
   class VirtRegMap : public MachineFunctionPass {
   public:
     enum {
-      NO_PHYS_REG = 0,
       NO_STACK_SLOT = (1L << 30)-1,
       MAX_STACK_SLOT = (1L << 18)-1
     };
@@ -49,7 +48,7 @@ class TargetInstrInfo;
     /// it; even spilled virtual registers (the register mapped to a
     /// spilled register is the temporary used to load it from the
     /// stack).
-    IndexedMap<Register, VirtReg2IndexFunctor> Virt2PhysMap;
+    IndexedMap<MCRegister, VirtReg2IndexFunctor> Virt2PhysMap;
 
     /// Virt2StackSlotMap - This is virtual register to stack slot
     /// mapping. Each spilled virtual register has an entry in it
@@ -71,9 +70,7 @@ class TargetInstrInfo;
   public:
     static char ID;
 
-    VirtRegMap()
-        : MachineFunctionPass(ID), Virt2PhysMap(NO_PHYS_REG),
-          Virt2StackSlotMap(NO_STACK_SLOT) {}
+    VirtRegMap() : MachineFunctionPass(ID), Virt2StackSlotMap(NO_STACK_SLOT) {}
     VirtRegMap(const VirtRegMap &) = delete;
     VirtRegMap &operator=(const VirtRegMap &) = delete;
 
@@ -96,15 +93,13 @@ class TargetInstrInfo;
 
     /// returns true if the specified virtual register is
     /// mapped to a physical register
-    bool hasPhys(Register virtReg) const {
-      return getPhys(virtReg) != NO_PHYS_REG;
-    }
+    bool hasPhys(Register virtReg) const { return getPhys(virtReg).isValid(); }
 
     /// returns the physical register mapped to the specified
     /// virtual register
     MCRegister getPhys(Register virtReg) const {
       assert(virtReg.isVirtual());
-      return MCRegister::from(Virt2PhysMap[virtReg]);
+      return Virt2PhysMap[virtReg];
     }
 
     /// creates a mapping for the specified virtual register to
@@ -130,9 +125,9 @@ class TargetInstrInfo;
     /// register mapping
     void clearVirt(Register virtReg) {
       assert(virtReg.isVirtual());
-      assert(Virt2PhysMap[virtReg] != NO_PHYS_REG &&
+      assert(Virt2PhysMap[virtReg] &&
              "attempt to clear a not assigned virtual register");
-      Virt2PhysMap[virtReg] = NO_PHYS_REG;
+      Virt2PhysMap[virtReg] = MCRegister();
     }
 
     /// clears all virtual to physical register mappings
@@ -178,8 +173,7 @@ class TargetInstrInfo;
         return true;
       // Split register can be assigned a physical register as well as a
       // stack slot or remat id.
-      return (Virt2SplitMap[virtReg] &&
-              Virt2PhysMap[virtReg] != NO_PHYS_REG);
+      return (Virt2SplitMap[virtReg] && Virt2PhysMap[virtReg]);
     }
 
     /// returns the stack slot mapped to the specified virtual
diff --git a/llvm/include/llvm/MC/MCRegister.h b/llvm/include/llvm/MC/MCRegister.h
index 530c1870abd6ad..dd8bc1e0f7189f 100644
--- a/llvm/include/llvm/MC/MCRegister.h
+++ b/llvm/include/llvm/MC/MCRegister.h
@@ -91,7 +91,6 @@ class MCRegister {
   /// Comparisons against register constants. E.g.
   /// * R == AArch64::WZR
   /// * R == 0
-  /// * R == VirtRegMap::NO_PHYS_REG
   constexpr bool operator==(unsigned Other) const { return Reg == Other; }
   constexpr bool operator!=(unsigned Other) const { return Reg != Other; }
   constexpr bool operator==(int Other) const { return Reg == unsigned(Other); }
diff --git a/llvm/lib/CodeGen/VirtRegMap.cpp b/llvm/lib/CodeGen/VirtRegMap.cpp
index e3b7da69df46b1..a548bf665bcea8 100644
--- a/llvm/lib/CodeGen/VirtRegMap.cpp
+++ b/llvm/lib/CodeGen/VirtRegMap.cpp
@@ -84,7 +84,7 @@ void VirtRegMap::grow() {
 
 void VirtRegMap::assignVirt2Phys(Register virtReg, MCPhysReg physReg) {
   assert(virtReg.isVirtual() && Register::isPhysicalRegister(physReg));
-  assert(Virt2PhysMap[virtReg] == NO_PHYS_REG &&
+  assert(!Virt2PhysMap[virtReg] &&
          "attempt to assign physical register to already mapped "
          "virtual register");
   assert(!getRegInfo().isReserved(physReg) &&
@@ -146,7 +146,7 @@ void VirtRegMap::print(raw_ostream &OS, const Module*) const {
   OS << "********** REGISTER MAP **********\n";
   for (unsigned i = 0, e = MRI->getNumVirtRegs(); i != e; ++i) {
     Register Reg = Register::index2VirtReg(i);
-    if (Virt2PhysMap[Reg] != (unsigned)VirtRegMap::NO_PHYS_REG) {
+    if (Virt2PhysMap[Reg]) {
       OS << '[' << printReg(Reg, TRI) << " -> "
          << printReg(Virt2PhysMap[Reg], TRI) << "] "
          << TRI->getRegClassName(MRI->getRegClass(Reg)) << "\n";
@@ -347,8 +347,8 @@ void VirtRegRewriter::addMBBLiveIns() {
       continue;
     // This is a virtual register that is live across basic blocks. Its
     // assigned PhysReg must be marked as live-in to those blocks.
-    Register PhysReg = VRM->getPhys(VirtReg);
-    if (PhysReg == VirtRegMap::NO_PHYS_REG) {
+    MCRegister PhysReg = VRM->getPhys(VirtReg);
+    if (!PhysReg) {
       // There may be no physical register assigned if only some register
       // classes were already allocated.
       assert(!ClearVirtRegs && "Unmapped virtual register");
@@ -551,7 +551,7 @@ void VirtRegRewriter::rewrite() {
           continue;
         Register VirtReg = MO.getReg();
         MCRegister PhysReg = VRM->getPhys(VirtReg);
-        if (PhysReg == VirtRegMap::NO_PHYS_REG)
+        if (!PhysReg)
           continue;
 
         assert(Register(PhysReg).isPhysical());
diff --git a/llvm/lib/Target/AMDGPU/SILowerWWMCopies.cpp b/llvm/lib/Target/AMDGPU/SILowerWWMCopies.cpp
index c6779659ff97fa..7bff58c7aa86c1 100644
--- a/llvm/lib/Target/AMDGPU/SILowerWWMCopies.cpp
+++ b/llvm/lib/Target/AMDGPU/SILowerWWMCopies.cpp
@@ -90,9 +90,8 @@ void SILowerWWMCopies::addToWWMSpills(MachineFunction &MF, Register Reg) {
   if (Reg.isPhysical())
     return;
 
-  Register PhysReg = VRM->getPhys(Reg);
-  assert(PhysReg != VirtRegMap::NO_PHYS_REG &&
-         "should have allocated a physical register");
+  MCRegister PhysReg = VRM->getPhys(Reg);
+  assert(PhysReg && "should have allocated a physical register");
 
   MFI->allocateWWMSpill(MF, PhysReg);
 }
diff --git a/llvm/lib/Target/X86/X86TileConfig.cpp b/llvm/lib/Target/X86/X86TileConfig.cpp
index 4552820f0f6240..30295e9929c6a3 100644
--- a/llvm/lib/Target/X86/X86TileConfig.cpp
+++ b/llvm/lib/Target/X86/X86TileConfig.cpp
@@ -128,9 +128,10 @@ bool X86TileConfig::runOnMachineFunction(MachineFunction &MF) {
       continue;
     if (MRI.getRegClass(VirtReg)->getID() != X86::TILERegClassID)
       continue;
-    if (VRM.getPhys(VirtReg) == VirtRegMap::NO_PHYS_REG)
+    MCRegister PhysReg = VRM.getPhys(VirtReg);
+    if (!PhysReg)
       continue;
-    unsigned Index = VRM.getPhys(VirtReg) - X86::TMM0;
+    unsigned Index = PhysReg - X86::TMM0;
     if (!Phys2Virt[Index])
       Phys2Virt[Index] = VirtReg;
   }

[arsenm]

If the key is a virtual register, this should still be Register

[topperc]

The Key type is determined by the argument_type field in VirtReg2IndexFunctor.

[github-actions]

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

You can test this locally with the following command:

git-clang-format --diff e523f4e2c3e20a843aeb79c412c1e8b3e90b96a3 1abb77b691f4ffb99b2c89f763d6713177b183e8 --extensions cpp,h -- llvm/include/llvm/CodeGen/Register.h llvm/include/llvm/CodeGen/VirtRegMap.h llvm/include/llvm/MC/MCRegister.h llvm/lib/CodeGen/VirtRegMap.cpp llvm/lib/Target/AMDGPU/SILowerWWMCopies.cpp llvm/lib/Target/X86/X86TileConfig.cpp

View the diff from clang-format here.

diff --git a/llvm/include/llvm/CodeGen/VirtRegMap.h b/llvm/include/llvm/CodeGen/VirtRegMap.h
index 96c28c191e..e2f9bd1d12 100644
--- a/llvm/include/llvm/CodeGen/VirtRegMap.h
+++ b/llvm/include/llvm/CodeGen/VirtRegMap.h
@@ -32,10 +32,7 @@ class TargetInstrInfo;
 
   class VirtRegMap : public MachineFunctionPass {
   public:
-    enum {
-      NO_STACK_SLOT = (1L << 30)-1,
-      MAX_STACK_SLOT = (1L << 18)-1
-    };
+    enum { NO_STACK_SLOT = (1L << 30) - 1, MAX_STACK_SLOT = (1L << 18) - 1 };
 
   private:
     MachineRegisterInfo *MRI = nullptr;