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

src/IceOperand.h

 //===- subzero/src/IceOperand.h - High-level operands -----------*- C++ -*-===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 384 matching lines @@
   }
 
   static bool classof(const Operand *Operand) {
     return Operand->getKind() == kConstUndef;
   }
 
 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.
 class RegWeight {
 public:
   RegWeight() = default;
   explicit RegWeight(uint32_t Weight) : Weight(Weight) {}
   RegWeight(const RegWeight &) = default;
   RegWeight &operator=(const RegWeight &) = default;
   constexpr static uint32_t Inf = ~0; /// Force regalloc to give a register
@@ skipping 112 matching lines @@
   bool getIgnoreLiveness() const { return IgnoreLiveness; }
 
   int32_t getStackOffset() const { return StackOffset; }
   void setStackOffset(int32_t Offset) { StackOffset = Offset; }
   /// Returns the variable's stack offset in symbolic form, to improve
   /// readability in DecorateAsm mode.
   IceString getSymbolicStackOffset(const Cfg *Func) const {
     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;
 
   void setMustHaveReg() { RegRequirement = RR_MustHaveRegister; }
   bool mustHaveReg() const { return RegRequirement == RR_MustHaveRegister; }
   void setMustNotHaveReg() { RegRequirement = RR_MustNotHaveRegister; }
   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);
     setMustHaveReg();
   }
   bool isRematerializable() const { return IsRematerializable; }
 
   void setRegClass(uint8_t RC) { RegisterClass = static_cast<RegClass>(RC); }
   RegClass getRegClass() const { return RegisterClass; }
 
@@ skipping 18 matching lines @@
     constexpr bool UseTrimmed = true;
     return Live.overlapsInst(Other->Live.getStart(), UseTrimmed);
   }
 
   /// Creates a temporary copy of the variable with a different type. Used
   /// primarily for syntactic correctness of textual assembly emission. Note
   /// that only basic information is copied, in particular not IsArgument,
   /// 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();
     return Kind >= kVariable && Kind <= kVariable_Max;
   }
 
 protected:
   Variable(OperandKind K, Type Ty, SizeT Index)
       : Operand(K, Ty), Number(Index),
         RegisterClass(static_cast<RegClass>(Ty)) {
@@ skipping 11 matching lines @@
   /// and validating live ranges. This is usually reserved for the stack
   /// pointer and other physical registers specifically referenced by name.
   bool IgnoreLiveness = false;
   // If IsRematerializable, RegNum keeps track of which register (stack or frame
   // pointer), and StackOffset is the known offset from that register.
   bool IsRematerializable = false;
   RegRequirement RegRequirement = RR_MayHaveRegister;
   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;
   LiveRange Live;
   /// VarsReal (and Operand::Vars) are set up such that Vars[0] == this.
   Variable *VarsReal[1];
 };
 
 // Variable64On32 represents a 64-bit variable on a 32-bit architecture. In
 // this situation the variable must be split into a low and a high word.
@@ skipping 164 matching lines @@
 private:
   const Cfg *Func;
   MetadataKind Kind;
   CfgVector<VariableTracking> Metadata;
   const static InstDefList NoDefinitions;
 };
 
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICEOPERAND_H