Implements include/exclude register lists for translation.

src/IceTargetLoweringARM32.cpp

Index: src/IceTargetLoweringARM32.cpp
diff --git a/src/IceTargetLoweringARM32.cpp b/src/IceTargetLoweringARM32.cpp
index 878f95ec059fbcf5c1e186cd4ff77b8d003cc0ad..d12a339fec0f0f1e24debaa52e8d31b47a87a6ae 100644
--- a/src/IceTargetLoweringARM32.cpp
+++ b/src/IceTargetLoweringARM32.cpp
@@ -1828,11 +1828,70 @@ Operand *TargetARM32::hiOperand(Operand *Operand) {
   return nullptr;
 }
 
+namespace {
+
+void applyUseRestrictedRegisters(
+    llvm::SmallBitVector &Registers,
+    const std::unordered_set<std::string> &RestrictedUses) {
+  assert(Registers.size() == RegARM32::Reg_NUM);
+
+  llvm::SmallBitVector GPKeepRegisters(RegARM32::Reg_NUM);

[Jim Stichnoth, 2016/01/10 16:51:25]

I think it might not be too hard to generalize this so that it's usable for
other architectures.

You can define a target-specific function that maps a register to a numeric
register class, such as:
  IsGPR ? 0 : IsFP32 ? 1 : IsFP64 ? 2 : IsVec128 ? 3 : 4
or:
  (IsGPR << 0) | (IsFP32 << 1) | (IsFP64 << 2) | (IsVec128 << 3)

Then either a std::vector<llvm::SmallBitVector> indexed by register class, or a
std::unordered_map<uint32_t, llvm::SmallBitVector> indexed by register class.

This target-specific mapping function could be passed in as a lambda.

In this model, probably the sense of e.g. NotGPRegisters has to change to
AllGPRegisters, and then
  Registers &= NotGPRegisters;
gets changed to
  Registers &= ~GPRegisters;
or the more efficient
  Registers.reset(GPRegisters);

I haven't worked this through completely so there's undoubtedly some yuckiness
lurking in trying to make such a general function properly templatized, or
otherwise with target-neutral arguments and data structures.  But it would be
great to have an initial implementation that doesn't call out 4 specific
register classes to prepare for extending this to x86.

[Karl, 2016/01/12 23:44:04]

Ok. Refactored the code to put this function in IceTargetLowering.{h,cpp} using
name getRegisterSetMask(). Then modified TargetARM32::staticInit() to call this
to build the set.

Also added to getRegisterSetMask() code to trace the available registers if the
corresponding verbose option is given.

+  llvm::SmallBitVector NotGPRegisters(RegARM32::Reg_NUM);
+  llvm::SmallBitVector SKeepRegisters(RegARM32::Reg_NUM);
+  llvm::SmallBitVector NotSRegisters(RegARM32::Reg_NUM);
+  llvm::SmallBitVector DKeepRegisters(RegARM32::Reg_NUM);
+  llvm::SmallBitVector NotDRegisters(RegARM32::Reg_NUM);
+  llvm::SmallBitVector VectorKeepRegisters(RegARM32::Reg_NUM);
+  llvm::SmallBitVector NotVectorRegisters(RegARM32::Reg_NUM);

[Jim Stichnoth, 2016/01/10 16:51:25]

Do we need IsI64Pair as well?  If so, there are some command line parsing issues
because those "register" names contain a comma...

[John, 2016/01/11 15:43:26]

I64 pairs should probably be turned off by disabling their individual parts --
just my 2cc.

[Karl, 2016/01/12 23:44:05]

Kept the register pairs, so that all registers can be defined. Fixed comma issue
by adding "RegNameFcn" that replace ", " with ":" in register names. This solves
the problem of how to specify on the command line.

+  for (size_t i = 0; i < RegARM32::Reg_NUM; ++i) {
+    const auto &Entry = RegARM32::Table[i];
+    NotGPRegisters[i] = !Entry.IsGPR;

[John, 2016/01/11 15:43:26]

Register pairs are not being handled here at all. As a matter of fact, any
register type that is not GPR, FP32, FP64, or Vec, are not being handled here
(they are incidentally because the Is??? features are mutually exclusive, but I
don't think depending on that is a good idea.)

optional: You could have something like



bool canBeUsed(int Register, const unordered_set<> &RestrictedRegisters) {
  return RestrictedUses.empty() || RestrictedUses.count(i) > 0;
}

bool isOther(int i) {
  const auto Register = RegARM32::Table[i];
  return !(Register.IsGPR | Register.IsFP32 || Register.IsFP64 ||
Register.IsVec128);
}

for (size_t i = 0; i < RegARM32::Reg_NUM; ++i) {
  SRegisterMask[i] =
     (IsFP32 && canBeUsed(i)) || IsFP64 || IsVec128 || IsGPR || isOther(i);
  DRegisterMask[i] =
     IsFP32 || (IsFP64 && canBeUsed(i)) || IsVec128 || IsGPR || isOther(i);
  QRegisterMask[i] =
     IsFP32 || IsFP64 || (IsVec128 && canBeUsed(i)) || IsGPR || isOther(i);
  GPRegisterMask[i] =
     IsFP32 || IsFP64 || IsVec128 || (IsGPR && canBeUsed(i)) || isOther(i);
}

Registers &=
    GPRegisterMask & SRegisterMask & DRegisterMask & QRegisterMask;

[Karl, 2016/01/12 23:44:04]

Fixed this by adding enum "RegClass" and static method getRegClass() to
IceRegistersARM32.h. This is then passed to the command line processor to
categorize what classes each register is in.  I added a "RegClass_Other" for any
register not explicitly defined in some other register class.

+    NotSRegisters[i] = !Entry.IsFP32;
+    NotDRegisters[i] = !Entry.IsFP64;
+    NotVectorRegisters[i] = !Entry.IsVec128;
+    if (RestrictedUses.find(Entry.Name) != RestrictedUses.end()) {
+      GPKeepRegisters[i] = Entry.IsGPR;
+      SKeepRegisters[i] = Entry.IsFP32;
+      DKeepRegisters[i] = Entry.IsFP64;
+      VectorKeepRegisters[i] = Entry.IsVec128;
+    }
+  }
+  if (GPKeepRegisters.any()) {
+    Registers &= NotGPRegisters;
+    Registers |= GPKeepRegisters;
+  }
+  if (SKeepRegisters.any()) {
+    Registers &= NotSRegisters;
+    Registers |= SKeepRegisters;
+  }
+  if (DKeepRegisters.any()) {
+    Registers &= NotDRegisters;
+    Registers |= DKeepRegisters;
+  }
+  if (VectorKeepRegisters.any()) {
+    Registers &= NotVectorRegisters;
+    Registers |= VectorKeepRegisters;
+  }
+}
+
+void applyExcludedRegisters(llvm::SmallBitVector &Registers,

[John, 2016/01/11 15:43:26]

pass registers by pointer (i.e., SmallBitVector *)

In the arm lowering I am trying to remove (and have already removed) non-const
references.

[Karl, 2016/01/12 23:44:05]

Removed. Now refactored into Ice::TargetLowering::getRegisterSetMask().

+                            const std::unordered_set<std::string> Excluded) {

[Jim Stichnoth, 2016/01/10 16:51:26]

&Excluded

[Karl, 2016/01/12 23:44:05]

No longer applicable.

+  for (size_t i = 0; i < RegARM32::Reg_NUM; ++i) {
+    const auto &Entry = RegARM32::Table[i];
+    if (Excluded.find(Entry.Name) != Excluded.end()) {

[John, 2016/01/11 15:43:26]

nit: this is less efficient than

Excluded.count(Entry.Name) > 0

[Karl, 2016/01/12 23:44:04]

Changed code to use method count rather than find.

+      if (Registers[i])
+        Registers[i] = 0;

[Jim Stichnoth, 2016/01/10 16:51:25]

Registers[i] = false;

Might as well remove the "if (Registers[i])" condition as well.

[Karl, 2016/01/12 23:44:05]

Acknowledged.

+    }
+  }
+}
+
+} // end of anonymous namespace
+
 llvm::SmallBitVector TargetARM32::getRegisterSet(RegSetMask Include,
                                                  RegSetMask Exclude) const {
   llvm::SmallBitVector Registers(RegARM32::Reg_NUM);
 
-  for (int i = 0; i < RegARM32::Reg_NUM; ++i) {
+  for (size_t i = 0; i < RegARM32::Reg_NUM; ++i) {

[John, 2016/01/11 15:43:26]

why? just curious...

[Karl, 2016/01/12 23:44:05]

Actually, I probably should have used SizeT. I did this because the index is
used to access arrays, and the type isn't int.

     const auto &Entry = RegARM32::Table[i];
     if (Entry.Scratch && (Include & RegSet_CallerSave))
       Registers[i] = true;
@@ -1852,6 +1911,16 @@ llvm::SmallBitVector TargetARM32::getRegisterSet(RegSetMask Include,
       Registers[i] = false;
   }
 
+  const std::unordered_set<std::string> &RestrictedUses =

[Jim Stichnoth, 2016/01/10 16:51:25]

A problem with calculating the restrictions here is that getRegisterSet is
called at least 2-4 times per translated method, and the restrictions never
change during translation.  (Fortunately the empty() calls probably keep it fast
for the common case.)

I think it would be better for staticInit() to calculate a mask which is ANDed
with Registers before returning.

[John, 2016/01/11 15:43:26]

s/RestrictedUses/RestrictedRegisters
s/ExcludedUses/ExcludedRegisters

[Karl, 2016/01/12 23:44:05]

Factored out into staticInit() that sets static field RegisterSetMask.

+      Ctx->getFlags().getUseRestrictedRegisters();
+  if (!RestrictedUses.empty())
+    applyUseRestrictedRegisters(Registers, RestrictedUses);
+
+  const std::unordered_set<std::string> &ExcludedUses =

[John, 2016/01/11 15:43:26]

I **really** wish we used `auto' here and above.

[Karl, 2016/01/12 23:44:05]

Acknowledged.

+      Ctx->getFlags().getExcludedRegisters();
+  if (!ExcludedUses.empty())
+    applyExcludedRegisters(Registers, ExcludedUses);
+
   return Registers;
 }