Implements include/exclude register lists for translation.

src/IceTargetLowering.cpp

Index: src/IceTargetLowering.cpp
diff --git a/src/IceTargetLowering.cpp b/src/IceTargetLowering.cpp
index 04f77e61b2caf26ea4fe4bedffc1efae35434fcd..36078377a708bf5f9d1476b8a3e16028602ce46b 100644
--- a/src/IceTargetLowering.cpp
+++ b/src/IceTargetLowering.cpp
@@ -39,7 +39,7 @@
 //       createTargetDataLowering(Ice::GlobalContext*);
 //   unique_ptr<Ice::TargetHeaderLowering>
 //       createTargetHeaderLowering(Ice::GlobalContext *);
-//   void staticInit(const ::Ice::ClFlags &Flags);
+//   void staticInit(::Ice::GlobalContext *);
 // }
 #define SUBZERO_TARGET(X)                                                      \
   namespace X {                                                                \
@@ -49,7 +49,7 @@
   createTargetDataLowering(::Ice::GlobalContext *Ctx);                         \
   std::unique_ptr<::Ice::TargetHeaderLowering>                                 \
   createTargetHeaderLowering(::Ice::GlobalContext *Ctx);                       \
-  void staticInit(const ::Ice::ClFlags &Flags);                                \
+  void staticInit(::Ice::GlobalContext *Ctx);                                  \
   } // end of namespace X
 #include "llvm/Config/SZTargets.def"
 #undef SUBZERO_TARGET
@@ -116,6 +116,95 @@ Variable *LoweringContext::availabilityGet(Operand *Src) const {
   return nullptr;
 }
 
+namespace {
+
+void indent(Ostream &Str, SizeT LineIndent) {
+  for (SizeT i = 0; i < LineIndent; ++i)
+    Str << ' ';
+}
+
+void printRegisterSet(Ostream &Str, const llvm::SmallBitVector &Bitset,
+                      std::function<IceString(int32_t)> getRegName,
+                      SizeT LineIndent) {
+  constexpr SizeT RegistersPerLine = 16;
+  SizeT Count = 0;
+  for (SizeT i = 0; i < Bitset.size(); ++i) {

[Jim Stichnoth, 2016/01/14 22:04:19]

The usual pattern for iterating over the set bits of a bitvector is:

  for (int i = Bitset.find_first(); i != -1; i = Bitset.find_next(i)) { ... }

[Karl, 2016/01/14 23:54:51]

Done.

+    if (!Bitset[i])
+      continue;
+    if (Count == 0) {
+      indent(Str, LineIndent);

[Jim Stichnoth, 2016/01/14 22:04:19]

I think you can remove the indent function, and just use:
  Str << std::string(LineIndent, ' ');

Or, just pass in the indent string (instead of its size) as an argument.

[Karl, 2016/01/14 23:54:51]

Done.

+    } else if (Count % RegistersPerLine == 0) {
+      Str << "\n";
+      indent(Str, LineIndent);
+    } else {
+      Str << ", ";

[Jim Stichnoth, 2016/01/14 22:04:19]

I would just print a space and drop the comma.  It's more compact, and besides
my OCD triggers when the linebreak omits the comma.

[Karl, 2016/01/14 23:54:51]

Done.

+    }
+    ++Count;
+    Str << getRegName(i);
+  }
+  if (Count)
+    Str << "\n";
+}
+
+} // end of anonymous namespace
+
+void TargetLowering::filterTypeToRegisterSet(
+    GlobalContext *Ctx, int32_t NumRegs,
+    llvm::SmallBitVector *TypeToRegisterSet, SizeT TypeToRegisterSetSize,
+    std::function<IceString(int32_t)> getRegName) {
+  llvm::SmallBitVector ExcludeBitSet(NumRegs);
+  std::vector<llvm::SmallBitVector> UseSet(TypeToRegisterSetSize,
+                                           ExcludeBitSet);
+  ExcludeBitSet.flip();
+
+  // Translate use/exclude name sets to corresponding bitsets.
+  const std::unordered_set<std::string> &UseName =

[John, 2016/01/14 21:12:49]

really great candidate for auto:

const auto &UseName

(if nothing else, use IceString instead of std::string)

[Karl, 2016/01/14 22:00:52]

Did change to IceString.

[Jim Stichnoth, 2016/01/14 22:04:19]

IceString

+      Ctx->getFlags().getUseRestrictedRegisters();
+  const std::unordered_set<std::string> &ExcludeName =
+      Ctx->getFlags().getExcludedRegisters();
+  for (int32_t RegIndex = 0; RegIndex < NumRegs; ++RegIndex) {

[John, 2016/01/14 21:12:48]

This loop iterates over all registers, but it only ever touches the ones in
UseName. Too bad you need RegIndex below. :(

[Karl, 2016/01/14 22:00:53]

Acknowledged.

+    IceString RegName(getRegName(RegIndex));

[John, 2016/01/14 21:12:48]

I usually prefer assignments when initializing string objects.

It also seems this object could be marked const.

[Karl, 2016/01/14 22:00:52]

I thought the form "clsname foo = bar;" Creates foo with the default
constructor, and then assigns. That is the main reason for not using assignment.
However, it is more readable. Changing...

+    if (UseName.count(RegName)) {

[Jim Stichnoth, 2016/01/14 22:04:19]

I realize UseName is const, but it would be extra nice if RegName could be
removed from UseName, and then after all processing is finished, any strings
left in UseName be bogus register names.  Those could be printed in an error
message and then pnacl-sz aborts.

Same for ExcludeName.

[Karl, 2016/01/14 23:54:51]

Done.

+      for (SizeT TypeIndex = 0; TypeIndex < TypeToRegisterSetSize;
+           ++TypeIndex) {
+        UseSet[TypeIndex][RegIndex] = TypeToRegisterSet[TypeIndex][RegIndex];
+      }
+    }
+    if (ExcludeName.count(RegName))
+      ExcludeBitSet[RegIndex] = false;
+  }
+
+  // Apply filters.
+  for (SizeT TypeIndex = 0; TypeIndex < TypeToRegisterSetSize; ++TypeIndex) {
+    llvm::SmallBitVector &TypeBitSet = TypeToRegisterSet[TypeIndex];

[John, 2016/01/14 21:12:48]

can you use a * here?

(yes, I know I am the only one who dislikes non-const references, but they
really help reading the code -- although there's an increase in * chars)

[Karl, 2016/01/14 22:00:52]

Done.

+    llvm::SmallBitVector &UseBitSet = UseSet[TypeIndex];
+    if (UseBitSet.any())
+      TypeBitSet = UseBitSet;
+    TypeBitSet &= ExcludeBitSet;
+  }
+
+  // Display filtered register sets, if requested.
+  if (BuildDefs::dump() && NumRegs &&
+      (Ctx->getFlags().getVerbose() & IceV_AvailableRegs)) {
+    Ostream &Str = Ctx->getStrEmit();

[Jim Stichnoth, 2016/01/14 22:04:19]

getStrDump

[Karl, 2016/01/14 23:54:51]

Done.

+    constexpr const char *LineIndent = "  ";

[John, 2016/01/14 21:12:49]

please use

constexpr char LineIndent = " ";

instead.

[Karl, 2016/01/14 22:00:52]

If I remove the const, the type is "char *const" (try it, the compiler will show
it). Hence, I can't remove the "const".

+    SizeT RegSetIndent = 2 * strlen(LineIndent);

[John, 2016/01/14 21:12:48]

and then using llvm::array_lengthof() should let you declare this as a constexpr
too.

[Karl, 2016/01/14 22:00:52]

Able to change using 'constexpr const char LineIndent[] = "  "'.

[Jim Stichnoth, 2016/01/14 22:04:19]

Can this be constexpr?  :)
(at least const, surely)

Also, I think size_t is more appropriate here, and for the printRegisterSet
parameter.

[Karl, 2016/01/14 23:54:51]

Done.

+    Str << "Registers available for register allocation:\n";
+    for (SizeT TypeIndex = 0; TypeIndex < TypeToRegisterSetSize; ++TypeIndex) {
+      Str << LineIndent;
+      if (TypeIndex < IceType_NUM) {
+        Str << typeString(static_cast<Type>(TypeIndex));
+      } else {
+        Str << "other[" << TypeIndex << "]";
+      }
+      Str << ":\n";
+      printRegisterSet(Str, TypeToRegisterSet[TypeIndex], getRegName,
+                       RegSetIndent);
+    }
+    Str << "\n";
+  }
+}
+
 std::unique_ptr<TargetLowering>
 TargetLowering::createLowering(TargetArch Target, Cfg *Func) {
   switch (Target) {
@@ -129,8 +218,8 @@ TargetLowering::createLowering(TargetArch Target, Cfg *Func) {
   }
 }
 
-void TargetLowering::staticInit(const ClFlags &Flags) {
-  const TargetArch Target = Flags.getTargetArch();
+void TargetLowering::staticInit(GlobalContext *Ctx) {
+  const TargetArch Target = Ctx->getFlags().getTargetArch();
   // Call the specified target's static initializer.
   switch (Target) {
   default:
@@ -142,7 +231,7 @@ void TargetLowering::staticInit(const ClFlags &Flags) {
       return;                                                                  \
     }                                                                          \
     InitGuard##X = true;                                                       \
-    ::X::staticInit(Flags);                                                    \
+    ::X::staticInit(Ctx);                                                      \
   } break;
 #include "llvm/Config/SZTargets.def"
 #undef SUBZERO_TARGET