Implements include/exclude register lists for translation.

src/IceTargetLowering.cpp

 //===- subzero/src/IceTargetLowering.cpp - Basic lowering implementation --===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 21 matching lines @@
 // (TargetLowering, TargetDataLowering, and TargetHeaderLowering) we use the
 // following named constructors. For reference, each target Foo needs to
 // implement the following named constructors and initializer:
 //
 // namespace Foo {
 //   unique_ptr<Ice::TargetLowering> createTargetLowering(Ice::Cfg *);
 //   unique_ptr<Ice::TargetDataLowering>
 //       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 {                                                                \
   std::unique_ptr<::Ice::TargetLowering>                                       \
   createTargetLowering(::Ice::Cfg *Func);                                      \
   std::unique_ptr<::Ice::TargetDataLowering>                                   \
   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
 
 namespace Ice {
 void LoweringContext::init(CfgNode *N) {
   Node = N;
   End = getNode()->getInsts().end();
   rewind();
   advanceForward(Next);
@@ skipping 46 matching lines @@
   LastSrc = llvm::cast<Variable>(Instr->getSrc(0));
 }
 
 Variable *LoweringContext::availabilityGet(Operand *Src) const {
   assert(Src);
   if (Src == LastDest)
     return LastSrc;
   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) {
   default:
     llvm::report_fatal_error("Unsupported target");
 #define SUBZERO_TARGET(X)                                                      \
   case Target_##X:                                                             \
     return ::X::createTargetLowering(Func);
 #include "llvm/Config/SZTargets.def"
 #undef SUBZERO_TARGET
   }
 }
 
-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:
     llvm::report_fatal_error("Unsupported target");
 #define SUBZERO_TARGET(X)                                                      \
   case Target_##X: {                                                           \
     static bool InitGuard##X = false;                                          \
     if (InitGuard##X) {                                                        \
       return;                                                                  \
     }                                                                          \
     InitGuard##X = true;                                                       \
-    ::X::staticInit(Flags);                                                    \
+    ::X::staticInit(Ctx);                                                      \
   } break;
 #include "llvm/Config/SZTargets.def"
 #undef SUBZERO_TARGET
   }
 }
 
 TargetLowering::TargetLowering(Cfg *Func)
     : Func(Func), Ctx(Func->getContext()), Context() {}
 
 void TargetLowering::genTargetHelperCalls() {
@@ skipping 526 matching lines @@
   case Target_##X:                                                             \
     return ::X::createTargetHeaderLowering(Ctx);
 #include "llvm/Config/SZTargets.def"
 #undef SUBZERO_TARGET
   }
 }
 
 TargetHeaderLowering::~TargetHeaderLowering() = default;
 
 } // end of namespace Ice