[NFCI] Cleanup range checks in Register/MCRegister

[daltenty]

Summary

by removing casts from unsigned to int that which may be implementation
defined according to C++14 (and thus trip up the XL compiler on AIX) by
just using unsigned comparisons/masks and refactor out the range
constants to cleanup things a bit while we are at it.

[hubert.reinterpretcast]

llvm/include/llvm/CodeGen/Register.h

36

Please use numeric_limits

• to catch if the type of Reg changes to in a way that the size no longer accurately reflects the range, and
• account for padding bits.

llvm/include/llvm/MC/MCRegister.h

38

Same comment.

[hubert.reinterpretcast]

llvm/include/llvm/CodeGen/Register.h

72

Note: We use the number interpretation here but then the bitmask interpretation below. Please see later comment.

79

I think we should stick purely with either the bitmask or the number interpretation. The name of MCRegister::FirstVirtualReg does not work well with the bitmask interpretation; however, since the bitmask operation is only valid given information that is not readily apparent to the compiler here (without adding a cast to uint32_t and changing the assert to llvm_unreachable), I guess the answer is to rename in favour of the bitmask interpretation. I'm using VirtualRegFlag in other comments for discussion purposes.

85

Should there be an assert that the "index" does not already have the indicator bit set?

llvm/include/llvm/MC/MCRegister.h

39

Would NullValue be correct?

52

To be consistent between using mask versus arithmetic:

!(Reg & VirtualRegFlag) && uint32_t(Reg) >= FirstSlackSlot;

if you want better code from GCC or

!(Reg & VirtualRegFlag) && uint32_t(Reg & ~VirtualRegFlag) >= FirstSlackSlot;

if you want better code from Clang.

59

Note: Number (and not bitmask) interpretation here.

[daltenty]

llvm/include/llvm/MC/MCRegister.h

39

Used NoRegister to be consistent with what TableGen calls this value.

[daltenty]

• Address comments round 1
  • Use numeric_limits for static assert
  • Use bit mask interpretation for VirtualRegFlag
  • Add range assert for index to virtual reg conversion

[hubert.reinterpretcast]

LGTM; thanks.

llvm/include/llvm/CodeGen/Register.h

public:
   //
   //   0           Not a register, can be used as a sentinel.
   //   [1;2^30)    Physical registers assigned by TableGen.
   //   [2^30;2^31) Stack slots. (Rarely used.)
   //   [2^31;2^32) Virtual registers assigned by MachineRegisterInfo.
   //
   // Further sentinels can be allocated from the small negative integers.
   // DenseMapInfo<unsigned> uses -1u and -2u.
+  static_assert(sizeof(Reg) >= 4, "Reg isn't large enough to hold full range.");
 
   /// isStackSlot - Sometimes it is useful the be able to store a non-negative
   /// frame index in a variable that normally holds a register. isStackSlot()
   /// returns true if Reg is in the range used for stack slots.
   ///
   /// Note that isVirtualRegister() and isPhysicalRegister() cannot handle stack
   /// slots, so if a variable may contains a stack slot, always check
   /// isStackSlot() first.
   ///
   static bool isStackSlot(unsigned Reg) {
     return MCRegister::isStackSlot(Reg);
   }
 
   /// Compute the frame index from a register value representing a stack slot.
   static int stackSlot2Index(unsigned Reg) {
     assert(isStackSlot(Reg) && "Not a stack slot");
-    return int(Reg - (1u << 30));
+    return int(Reg - MCRegister::FirstStackSlot);
   }
 
   /// Convert a non-negative frame index to a stack slot register value.
   static unsigned index2StackSlot(int FI) {
     assert(FI >= 0 && "Cannot hold a negative frame index.");
-    return FI + (1u << 30);
+    return FI + MCRegister::FirstStackSlot;
   }
 
   /// Return true if the specified register number is in
   /// the physical register namespace.
   static bool isPhysicalRegister(unsigned Reg) {
     return MCRegister::isPhysicalRegister(Reg);
   }
 
   /// Return true if the specified register number is in
   /// the virtual register namespace.
   static bool isVirtualRegister(unsigned Reg) {
     assert(!isStackSlot(Reg) && "Not a register! Check isStackSlot() first.");
-    return int(Reg) < 0;
+    return Reg >= MCRegister::FirstVirtualReg;
   }
 
   /// Convert a virtual register number to a 0-based index.
   /// The first virtual register in a function will get the index 0.
   static unsigned virtReg2Index(unsigned Reg) {
     assert(isVirtualRegister(Reg) && "Not a virtual register");
-    return Reg & ~(1u << 31);
+    return Reg & ~MCRegister::FirstVirtualReg;
   }
 
   /// Convert a 0-based index to a virtual register number.
   /// This is the inverse operation of VirtReg2IndexFunctor below.
   static unsigned index2VirtReg(unsigned Index) {
-    return Index | (1u << 31);
+    return Index | MCRegister::FirstVirtualReg;
   }
 
   /// Return true if the specified register number is in the virtual register
   /// namespace.
   bool isVirtual() const {
     return isVirtualRegister(Reg);
   }
 
public:
   }
 
   unsigned id() const { return Reg; }
 
   operator MCRegister() const {
     return MCRegister(Reg);
   }
 
-  bool isValid() const {
-    return Reg != 0;
-  }
+  bool isValid() const { return Reg != MCRegister::NoRegister; }
 
   /// Comparisons between register objects
   bool operator==(const Register &Other) const { return Reg == Other.Reg; }
   bool operator!=(const Register &Other) const { return Reg != Other.Reg; }
   bool operator==(const MCRegister &Other) const { return Reg == Other.id(); }
   bool operator!=(const MCRegister &Other) const { return Reg != Other.id(); }
 
   /// Comparisons against register constants. E.g.

llvm/include/llvm/MC/MCRegister.h

public:
   //
   //   0           Not a register, can be used as a sentinel.
   //   [1;2^30)    Physical registers assigned by TableGen.
   //   [2^30;2^31) Stack slots. (Rarely used.)
   //   [2^31;2^32) Virtual registers assigned by MachineRegisterInfo.
   //
   // Further sentinels can be allocated from the small negative integers.
   // DenseMapInfo<unsigned> uses -1u and -2u.
+  static_assert(sizeof(Reg) >= 4, "Reg isn't large enough to hold full range.");
+  static constexpr unsigned NoRegister = 0u;
+  static constexpr unsigned FirstPhysicalReg = 1u;
+  static constexpr unsigned FirstStackSlot = 1u << 30;
+  static constexpr unsigned FirstVirtualReg = 1u << 31;
 
   /// This is the portion of the positive number space that is not a physical
   /// register. StackSlot values do not exist in the MC layer, see
   /// Register::isStackSlot() for the more information on them.
   ///
   /// Note that isVirtualRegister() and isPhysicalRegister() cannot handle stack
   /// slots, so if a variable may contains a stack slot, always check
   /// isStackSlot() first.
   static bool isStackSlot(unsigned Reg) {
-    return int(Reg) >= (1 << 30);
+    return (Reg & FirstStackSlot) && Reg < FirstVirtualReg;
   }
 
   /// Return true if the specified register number is in
   /// the physical register namespace.
   static bool isPhysicalRegister(unsigned Reg) {
     assert(!isStackSlot(Reg) && "Not a register! Check isStackSlot() first.");
-    return int(Reg) > 0;
+    return Reg >= FirstPhysicalReg && Reg < FirstVirtualReg;
   }
 
   /// Return true if the specified register number is in the physical register
   /// namespace.
   bool isPhysical() const {
     return isPhysicalRegister(Reg);
   }
 
   constexpr operator unsigned() const {
     return Reg;
   }
 
   unsigned id() const {
     return Reg;
   }
 
-  bool isValid() const {
-    return Reg != 0;
-  }
+  bool isValid() const { return Reg != NoRegister; }
 
   /// Comparisons between register objects
   bool operator==(const MCRegister &Other) const { return Reg == Other.Reg; }
   bool operator!=(const MCRegister &Other) const { return Reg != Other.Reg; }
 
   /// Comparisons against register constants. E.g.
   /// * R == AArch64::WZR
   /// * R == 0