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

src/IceTargetLoweringX8664Traits.h

 //===- subzero/src/IceTargetLoweringX8664Traits.h - x86-64 traits -*- C++ -*-=//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 283 matching lines @@
     Begin,
     // SSE2 is the PNaCl baseline instruction set.
     SSE2 = Begin,
     SSE4_1,
     End
   };
 
   static const char *TargetName;
   static constexpr Type WordType = IceType_i64;
 
-  static IceString getRegName(int32_t RegNum) {
+  static inline void assertValidRegNum(RegNumT RegNum) {

[Eric Holk, 2016/02/08 19:37:10]

This is the third time I've seen this function. It seems like we should move it
to a common location.

[Jim Stichnoth, 2016/02/09 19:33:40]

That could be tricky given that the upper bound is target-specific.

[Eric Holk, 2016/02/10 01:11:30]

Ah, right. It does make me think there might be another way to factor this, but
maybe you've tried that in some of your later changes.

+    (void)RegNum;
+    assert(RegNum != RegNumT::NoRegister);
+    assert(unsigned(RegNum) < RegisterSet::Reg_NUM);
+  }
+
+  static IceString getRegName(RegNumT RegNum) {
     static const char *const RegNames[RegisterSet::Reg_NUM] = {
 #define X(val, encode, name, base, scratch, preserved, stackptr, frameptr,     \
           sboxres, isGPR, is64, is32, is16, is8, isXmm, is64To8, is32To8,      \
           is16To8, isTrunc8Rcvr, isAhRcvr, aliases)                            \
   name,
         REGX8664_TABLE
 #undef X
     };
-    assert(RegNum >= 0);
-    assert(RegNum < RegisterSet::Reg_NUM);
+    assertValidRegNum(RegNum);
     return RegNames[RegNum];
   }
 
-  static GPRRegister getEncodedGPR(int32_t RegNum) {
+  static GPRRegister getEncodedGPR(RegNumT RegNum) {
     static const GPRRegister GPRRegs[RegisterSet::Reg_NUM] = {
 #define X(val, encode, name, base, scratch, preserved, stackptr, frameptr,     \
           sboxres, isGPR, is64, is32, is16, is8, isXmm, is64To8, is32To8,      \
           is16To8, isTrunc8Rcvr, isAhRcvr, aliases)                            \
   GPRRegister(isGPR ? encode : GPRRegister::Encoded_Not_GPR),
         REGX8664_TABLE
 #undef X
     };
-    assert(RegNum >= 0);
-    assert(RegNum < RegisterSet::Reg_NUM);
+    assertValidRegNum(RegNum);
     assert(GPRRegs[RegNum] != GPRRegister::Encoded_Not_GPR);
     return GPRRegs[RegNum];
   }
 
-  static ByteRegister getEncodedByteReg(int32_t RegNum) {
+  static ByteRegister getEncodedByteReg(RegNumT RegNum) {
     static const ByteRegister ByteRegs[RegisterSet::Reg_NUM] = {
 #define X(val, encode, name, base, scratch, preserved, stackptr, frameptr,     \
           sboxres, isGPR, is64, is32, is16, is8, isXmm, is64To8, is32To8,      \
           is16To8, isTrunc8Rcvr, isAhRcvr, aliases)                            \
   ByteRegister(is8 ? encode : ByteRegister::Encoded_Not_ByteReg),
         REGX8664_TABLE
 #undef X
     };
-    assert(RegNum >= 0);
-    assert(RegNum < RegisterSet::Reg_NUM);
+    assertValidRegNum(RegNum);
     assert(ByteRegs[RegNum] != ByteRegister::Encoded_Not_ByteReg);
     return ByteRegs[RegNum];
   }
 
-  static XmmRegister getEncodedXmm(int32_t RegNum) {
+  static XmmRegister getEncodedXmm(RegNumT RegNum) {
     static const XmmRegister XmmRegs[RegisterSet::Reg_NUM] = {
 #define X(val, encode, name, base, scratch, preserved, stackptr, frameptr,     \
           sboxres, isGPR, is64, is32, is16, is8, isXmm, is64To8, is32To8,      \
           is16To8, isTrunc8Rcvr, isAhRcvr, aliases)                            \
   XmmRegister(isXmm ? encode : XmmRegister::Encoded_Not_Xmm),
         REGX8664_TABLE
 #undef X
     };
-    assert(RegNum >= 0);
-    assert(RegNum < RegisterSet::Reg_NUM);
+    assertValidRegNum(RegNum);
     assert(XmmRegs[RegNum] != XmmRegister::Encoded_Not_Xmm);
     return XmmRegs[RegNum];
   }
 
-  static uint32_t getEncoding(int32_t RegNum) {
+  static uint32_t getEncoding(RegNumT RegNum) {
     static const uint32_t Encoding[RegisterSet::Reg_NUM] = {
 #define X(val, encode, name, base, scratch, preserved, stackptr, frameptr,     \
           sboxres, isGPR, is64, is32, is16, is8, isXmm, is64To8, is32To8,      \
           is16To8, isTrunc8Rcvr, isAhRcvr, aliases)                            \
   encode,
         REGX8664_TABLE
 #undef X
     };
-    assert(RegNum >= 0);
-    assert(RegNum < RegisterSet::Reg_NUM);
+    assertValidRegNum(RegNum);
     return Encoding[RegNum];
   }
 
-  static inline int32_t getBaseReg(int32_t RegNum) {
-    static const int32_t BaseRegs[RegisterSet::Reg_NUM] = {
+  static inline RegNumT getBaseReg(RegNumT RegNum) {
+    static const RegNumT BaseRegs[RegisterSet::Reg_NUM] = {
 #define X(val, encode, name, base, scratch, preserved, stackptr, frameptr,     \
           sboxres, isGPR, is64, is32, is16, is8, isXmm, is64To8, is32To8,      \
           is16To8, isTrunc8Rcvr, isAhRcvr, aliases)                            \
-  encode,
+  RegisterSet::base,

[Eric Holk, 2016/02/08 19:37:10]

Was this a bug previously? This change looks like it has different behavior than
before.

[Jim Stichnoth, 2016/02/09 19:33:40]

Yes, it was a bug, but shockingly it didn't affect at least the spec2k
translation.

[Eric Holk, 2016/02/10 01:11:31]

Huh. Good thing we caught it.

         REGX8664_TABLE
 #undef X
     };
-    assert(RegNum >= 0);
-    assert(RegNum < RegisterSet::Reg_NUM);
+    assertValidRegNum(RegNum);
     return BaseRegs[RegNum];
   }
 
 private:
-  static int32_t getFirstGprForType(Type Ty) {
+  static RegNumT getFirstGprForType(Type Ty) {
     switch (Ty) {
     default:
       llvm_unreachable("Invalid type for GPR.");
     case IceType_i1:
     case IceType_i8:
       return RegisterSet::Reg_al;
     case IceType_i16:
       return RegisterSet::Reg_ax;
     case IceType_i32:
       return RegisterSet::Reg_eax;
     case IceType_i64:
       return RegisterSet::Reg_rax;
     }
   }
 
 public:
-  static int32_t getGprForType(Type Ty, int32_t RegNum) {
-    assert(RegNum != Variable::NoRegister);
+  static RegNumT getGprForType(Type Ty, RegNumT RegNum) {
+    assert(RegNum != RegNumT::NoRegister);
 
     if (!isScalarIntegerType(Ty)) {
       return RegNum;
     }
 
     assert(Ty == IceType_i1 || Ty == IceType_i8 || Ty == IceType_i16 ||
            Ty == IceType_i32 || Ty == IceType_i64);
 
     if (RegNum == RegisterSet::Reg_ah) {
       assert(Ty == IceType_i8);
       return RegNum;
     }
 
     assert(RegNum != RegisterSet::Reg_bh);
     assert(RegNum != RegisterSet::Reg_ch);
     assert(RegNum != RegisterSet::Reg_dh);
 
-    const int32_t FirstGprForType = getFirstGprForType(Ty);
+    const RegNumT FirstGprForType = getFirstGprForType(Ty);
 
     switch (RegNum) {
     default:
       llvm::report_fatal_error("Unknown register.");
 #define X(val, encode, name, base, scratch, preserved, stackptr, frameptr,     \
           sboxres, isGPR, is64, is32, is16, is8, isXmm, is64To8, is32To8,      \
           is16To8, isTrunc8Rcvr, isAhRcvr, aliases)                            \
   case RegisterSet::val: {                                                     \
     if (!isGPR)                                                                \
       return RegisterSet::val;                                                 \
     assert((is64) || (is32) || (is16) || (is8) ||                              \
            getBaseReg(RegisterSet::val) == RegisterSet::Reg_rsp);              \
-    constexpr int32_t FirstGprWithRegNumSize =                                 \
+    constexpr RegisterSet::AllRegisters FirstGprWithRegNumSize =               \
         ((is64) || RegisterSet::val == RegisterSet::Reg_rsp)                   \
             ? RegisterSet::Reg_rax                                             \
             : (((is32) || RegisterSet::val == RegisterSet::Reg_esp)            \
                    ? RegisterSet::Reg_eax                                      \
                    : (((is16) || RegisterSet::val == RegisterSet::Reg_sp)      \
                           ? RegisterSet::Reg_ax                                \
                           : RegisterSet::Reg_al));                             \
-    const int32_t NewRegNum =                                                  \
-        RegNum - FirstGprWithRegNumSize + FirstGprForType;                     \
+    const RegNumT NewRegNum =                                                  \
+        RegNumT::fixme(RegNum - FirstGprWithRegNumSize + FirstGprForType);     \
     assert(getBaseReg(RegNum) == getBaseReg(NewRegNum) &&                      \
            "Error involving " #val);                                           \
     return NewRegNum;                                                          \
   }
       REGX8664_TABLE
 #undef X
     }
   }
 
 private:
@@ skipping 156 matching lines @@
 
     REGX8664_TABLE
 
 #undef X
 
     return Registers;
   }
 
   static void
   makeRandomRegisterPermutation(GlobalContext *Ctx, Cfg *Func,
-                                llvm::SmallVectorImpl<int32_t> &Permutation,
+                                llvm::SmallVectorImpl<RegNumT> &Permutation,
                                 const llvm::SmallBitVector &ExcludeRegisters,
                                 uint64_t Salt) {
     // TODO(stichnot): Declaring Permutation this way loses type/size
     // information. Fix this in conjunction with the caller-side TODO.
     assert(Permutation.size() >= RegisterSet::Reg_NUM);
     // Expected upper bound on the number of registers in a single equivalence
     // class.  For x86-64, this would comprise the 16 XMM registers. This is
     // for performance, not correctness.
     static const unsigned MaxEquivalenceClassSize = 8;
-    using RegisterList = llvm::SmallVector<int32_t, MaxEquivalenceClassSize>;
+    using RegisterList = llvm::SmallVector<RegNumT, MaxEquivalenceClassSize>;
     using EquivalenceClassMap = std::map<uint32_t, RegisterList>;
     EquivalenceClassMap EquivalenceClasses;
     SizeT NumShuffled = 0, NumPreserved = 0;
 
 // Build up the equivalence classes of registers by looking at the register
 // properties as well as whether the registers should be explicitly excluded
 // from shuffling.
 #define X(val, encode, name, base, scratch, preserved, stackptr, frameptr,     \
           sboxres, isGPR, is64, is32, is16, is8, isXmm, is64To8, is32To8,      \
           is16To8, isTrunc8Rcvr, isAhRcvr, aliases)                            \
@@ skipping 41 matching lines @@
     assert(NumShuffled + NumPreserved == RegisterSet::Reg_NUM);
 
     if (Func->isVerbose(IceV_Random)) {
       OstreamLocker L(Func->getContext());
       Ostream &Str = Func->getContext()->getStrDump();
       Str << "Register equivalence classes:\n";
       for (auto I : EquivalenceClasses) {
         Str << "{";
         const RegisterList &List = I.second;
         bool First = true;
-        for (int32_t Register : List) {
+        for (RegNumT Register : List) {
           if (!First)
             Str << " ";
           First = false;
           Str << getRegName(Register);
         }
         Str << "}\n";
       }
     }
   }
 
-  static int32_t getRaxOrDie() { return RegisterSet::Reg_rax; }
+  static RegNumT getRaxOrDie() { return RegisterSet::Reg_rax; }
 
-  static int32_t getRdxOrDie() { return RegisterSet::Reg_rdx; }
+  static RegNumT getRdxOrDie() { return RegisterSet::Reg_rdx; }
 
   // x86-64 calling convention:
   //
   // * The first eight arguments of vector/fp type, regardless of their
   // position relative to the other arguments in the argument list, are placed
   // in registers %xmm0 - %xmm7.
   //
   // * The first six arguments of integer types, regardless of their position
   // relative to the other arguments in the argument list, are placed in
   // registers %rdi, %rsi, %rdx, %rcx, %r8, and %r9.
   //
   // This intends to match the section "Function Calling Sequence" of the
   // document "System V Application Binary Interface."
 
   /// The maximum number of arguments to pass in XMM registers
   static constexpr uint32_t X86_MAX_XMM_ARGS = 8;
   /// The maximum number of arguments to pass in GPR registers
   static constexpr uint32_t X86_MAX_GPR_ARGS = 6;
   /// Whether scalar floating point arguments are passed in XMM registers
   static constexpr bool X86_PASS_SCALAR_FP_IN_XMM = true;
   /// Get the register for a given argument slot in the XMM registers.
-  static int32_t getRegisterForXmmArgNum(uint32_t ArgNum) {
+  static RegNumT getRegisterForXmmArgNum(uint32_t ArgNum) {
     // TODO(sehr): Change to use the CCArg technique used in ARM32.
     static_assert(RegisterSet::Reg_xmm0 + 1 == RegisterSet::Reg_xmm1,
                   "Inconsistency between XMM register numbers and ordinals");
     if (ArgNum >= X86_MAX_XMM_ARGS) {
-      return Variable::NoRegister;
+      return RegNumT::NoRegister;
     }
-    return static_cast<int32_t>(RegisterSet::Reg_xmm0 + ArgNum);
+    return RegNumT::fixme(RegisterSet::Reg_xmm0 + ArgNum);
   }
   /// Get the register for a given argument slot in the GPRs.
-  static int32_t getRegisterForGprArgNum(Type Ty, uint32_t ArgNum) {
+  static RegNumT getRegisterForGprArgNum(Type Ty, uint32_t ArgNum) {
     if (ArgNum >= X86_MAX_GPR_ARGS) {
-      return Variable::NoRegister;
+      return RegNumT::NoRegister;
     }
     static const RegisterSet::AllRegisters GprForArgNum[] = {
         RegisterSet::Reg_rdi, RegisterSet::Reg_rsi, RegisterSet::Reg_rdx,
         RegisterSet::Reg_rcx, RegisterSet::Reg_r8,  RegisterSet::Reg_r9,
     };
     static_assert(llvm::array_lengthof(GprForArgNum) == X86_MAX_GPR_ARGS,
                   "Mismatch between MAX_GPR_ARGS and GprForArgNum.");
     assert(Ty == IceType_i64 || Ty == IceType_i32);
-    return static_cast<int32_t>(getGprForType(Ty, GprForArgNum[ArgNum]));
+    return getGprForType(Ty, GprForArgNum[ArgNum]);

[Eric Holk, 2016/02/08 19:37:10]

I love how many casts this change is getting rid of!

[Jim Stichnoth, 2016/02/09 19:33:40]

Acknowledged.

   }
 
   /// The number of bits in a byte
   static constexpr uint32_t X86_CHAR_BIT = 8;
   /// Stack alignment. This is defined in IceTargetLoweringX8664.cpp because it
   /// is used as an argument to std::max(), and the default std::less<T> has an
   /// operator(T const&, T const&) which requires this member to have an
   /// address.
   static const uint32_t X86_STACK_ALIGNMENT_BYTES;
   /// Size of the return address on the stack
@@ skipping 272 matching lines @@
     const char *FldString;   // s, l, or <blank>
   } TypeAttributes[];
 };
 
 using Traits = ::Ice::X8664::TargetX8664Traits;
 } // end of namespace X8664
 
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICETARGETLOWERINGX8664TRAITS_H