Subzero: Use a proper RegNumT type instead of int32_t/SizeT.

src/IceOperand.h

Index: src/IceOperand.h
diff --git a/src/IceOperand.h b/src/IceOperand.h
index 91e9de0f3be83633cf5638664f972907083f0576..2b0a9bc977642d52c8f61b29a5e4aa3b38e2f716 100644
--- a/src/IceOperand.h
+++ b/src/IceOperand.h
@@ -402,6 +402,116 @@ private:
   ConstantUndef(Type Ty) : Constant(kConstUndef, Ty) {}
 };
 
+/// RegNumT is for holding target-specific register numbers, plus the sentinel
+/// value NoRegister.  Its public ctor allows direct use of enum values, such as
+/// RegNumT(Reg_eax), but not things like RegNumT(Reg_eax+1).  This is to try to
+/// prevent inappropriate assumptions about enum ordering.  If needed, the
+/// fromInt() method can be used, such as when a RegNumT is based on a bitvector
+/// index.
+class RegNumT {
+public:
+  using BaseType = uint32_t;
+  RegNumT() = default;
+  RegNumT(const RegNumT &) = default;
+  template <typename AnyEnum>
+  RegNumT(AnyEnum Value,
+          typename std::enable_if<std::is_enum<AnyEnum>::value, int>::type = 0)
+      : Value(Value) {
+    validate(Value);
+  }
+  RegNumT &operator=(const RegNumT &) = default;
+  operator unsigned() const { return Value; }
+  /// Asserts that the register is valid, i.e. not NoRegister.  Note that the
+  /// ctor already does the target-specific limit check.
+  void assertIsValid() const { assert(Value != NoRegister); }
+  static RegNumT fromInt(BaseType Value) { return RegNumT(Value); }
+  /// Marks cases that inappropriately add/subtract RegNumT values, and
+  /// therefore need to be fixed because they make assumptions about register
+  /// enum value ordering.  TODO(stichnot): Remove fixme() as soon as all
+  /// current uses are fixed/removed.
+  static RegNumT fixme(BaseType Value) { return RegNumT(Value); }
+  /// The target's staticInit() method should call setLimit() to register the
+  /// upper bound of allowable values.
+  static void setLimit(BaseType Value) {
+    // Make sure it's only called once.
+    assert(Limit == 0);
+    assert(Value != 0);
+    Limit = Value;
+  }
+  // Define NoRegister as an enum value so that it can be used as an argument

[John, 2016/02/10 16:01:51]

nice. :)

[Jim Stichnoth, 2016/02/10 17:47:20]

True, though that inhibits certain uses of "auto", e.g.:
  auto RegNum = RegNumT::NoRegister;
when we want RegNum to have type RegNumT.

I was thinking of something like:

  enum { NoRegisterValue = std::numeric_limits<BaseType>::max() };
  const static RegNumT NoRegister /* = NoRegisterValue */ ;

but I am le tired

[Jim Stichnoth, 2016/02/10 18:47:30]

I am now le refreshed, and realize the horror of the compiler error messages
that propagate from "auto RegNum = RegNumT::NoRegister;", so I went ahead and
made that change.

+  // for the public ctor.
+  enum { NoRegister = std::numeric_limits<BaseType>::max() };
+
+private:
+  BaseType Value = NoRegister;
+  static BaseType Limit;
+  /// Private ctor called only by fromInt() and fixme().
+  RegNumT(BaseType Value) : Value(Value) { validate(Value); }
+  /// The ctor calls this to validate against the target-supplied limit.
+  static void validate(BaseType Value) {
+    (void)Value;
+    assert(Value == NoRegister || Value < Limit);
+  }
+  /// Disallow operators that inappropriately make assumptions about register
+  /// enum value ordering.
+  bool operator<(const RegNumT &) = delete;
+  bool operator<=(const RegNumT &) = delete;
+  bool operator>(const RegNumT &) = delete;
+  bool operator>=(const RegNumT &) = delete;
+};
+
+/// RegNumBitVector wraps llvm::SmallBitVector so that instead of this pattern:
+///
+///   for (int i = V.find_first(); i != -1; i = V.find_next(i)) {
+///     RegNumT RegNum = RegNumT::fromInt(i);
+///     ...
+///   }
+///
+/// this cleaner pattern can be used:
+///
+///   for (RegNumT RegNum : RegNumBitVector(V)) {
+///     ...
+///   }
+class RegNumBitVector {

[John, 2016/02/10 16:01:51]

This is an iterator, so maybe just rename it to RegNumBitVectorIter(ator)?

[Jim Stichnoth, 2016/02/10 17:47:20]

I was trying to control the identifier length a bit. :)

I will try on "RegNumBVIter", I think that captures everything while being
compact.

BTW, I really wanted something like this:

  template <class T = llvm::SmallBitVector> class RegNumBVIter { ... }

because why not allow it for llvm::BitVector as well, but then you end up with
the clumsy

  for (RegNumT RegNum : RegNumBVIter<>(Mask)) { ... }

instead.

+  using T = llvm::SmallBitVector;
+  static constexpr int Sentinel = -1;
+  RegNumBitVector() = delete;
+  RegNumBitVector(const RegNumBitVector &) = delete;
+  RegNumBitVector &operator=(const RegNumBitVector &) = delete;
+
+public:
+  class Iterator {
+    Iterator() = delete;
+    Iterator &operator=(const Iterator &) = delete;
+
+  public:
+    explicit Iterator(const T &V) : V(V), Current(V.find_first()) {}
+    Iterator(const T &V, int Value) : V(V), Current(Value) {}
+    Iterator(const Iterator &) = default;
+    RegNumT operator*() {
+      assert(Current != Sentinel);
+      return RegNumT::fromInt(Current);
+    }
+    Iterator &operator++() {
+      assert(Current != Sentinel);
+      Current = V.find_next(Current);
+      return *this;
+    }
+    bool operator!=(Iterator &Other) { return Current != Other.Current; }
+
+  private:
+    const T &V;
+    int Current;
+  };
+
+  explicit RegNumBitVector(const T &V) : V(V) {}
+  Iterator begin() { return Iterator(V); }
+  Iterator end() { return Iterator(V, Sentinel); }
+
+private:
+  const T &V;
+};
+
 /// RegWeight is a wrapper for a uint32_t weight value, with a special value
 /// that represents infinite weight, and an addWeight() method that ensures that
 /// W+infinity=infinity.
@@ -534,17 +644,16 @@ public:
     return "lv$" + getName(Func);
   }
 
-  static constexpr int32_t NoRegister = -1;
-  bool hasReg() const { return getRegNum() != NoRegister; }
-  int32_t getRegNum() const { return RegNum; }
-  void setRegNum(int32_t NewRegNum) {
+  bool hasReg() const { return getRegNum() != RegNumT::NoRegister; }
+  RegNumT getRegNum() const { return RegNum; }
+  void setRegNum(RegNumT NewRegNum) {
     // Regnum shouldn't be set more than once.
     assert(!hasReg() || RegNum == NewRegNum);
     RegNum = NewRegNum;
   }
-  bool hasRegTmp() const { return getRegNumTmp() != NoRegister; }
-  int32_t getRegNumTmp() const { return RegNumTmp; }
-  void setRegNumTmp(int32_t NewRegNum) { RegNumTmp = NewRegNum; }
+  bool hasRegTmp() const { return getRegNumTmp() != RegNumT::NoRegister; }
+  RegNumT getRegNumTmp() const { return RegNumTmp; }
+  void setRegNumTmp(RegNumT NewRegNum) { RegNumTmp = NewRegNum; }
 
   RegWeight getWeight(const Cfg *Func) const;
 
@@ -554,7 +663,7 @@ public:
   bool mustNotHaveReg() const {
     return RegRequirement == RR_MustNotHaveRegister;
   }
-  void setRematerializable(int32_t NewRegNum, int32_t NewOffset) {
+  void setRematerializable(RegNumT NewRegNum, int32_t NewOffset) {
     IsRematerializable = true;
     setRegNum(NewRegNum);
     setStackOffset(NewOffset);
@@ -593,14 +702,14 @@ public:
   /// IsImplicitArgument, IgnoreLiveness, RegNumTmp, Live, LoVar, HiVar,
   /// VarsReal.  If NewRegNum!=NoRegister, then that register assignment is made
   /// instead of copying the existing assignment.
-  const Variable *asType(Type Ty, int32_t NewRegNum) const;
+  const Variable *asType(Type Ty, RegNumT NewRegNum) const;
 
   void emit(const Cfg *Func) const override;
   using Operand::dump;
   void dump(const Cfg *Func, Ostream &Str) const override;
 
   /// Return reg num of base register, if different from stack/frame register.
-  virtual int32_t getBaseRegNum() const { return NoRegister; }
+  virtual RegNumT getBaseRegNum() const { return RegNumT::NoRegister; }
 
   static bool classof(const Operand *Operand) {
     OperandKind Kind = Operand->getKind();
@@ -632,9 +741,9 @@ protected:
   RegClass RegisterClass;
   /// RegNum is the allocated register, or NoRegister if it isn't
   /// register-allocated.
-  int32_t RegNum = NoRegister;
+  RegNumT RegNum = RegNumT::NoRegister;
   /// RegNumTmp is the tentative assignment during register allocation.
-  int32_t RegNumTmp = NoRegister;
+  RegNumT RegNumTmp = RegNumT::NoRegister;
   /// StackOffset is the canonical location on stack (only if
   /// RegNum==NoRegister || IsArgument).
   int32_t StackOffset = 0;