Clean up register+immediate addresses in ARM.

src/IceAssemblerARM32.cpp

Index: src/IceAssemblerARM32.cpp
diff --git a/src/IceAssemblerARM32.cpp b/src/IceAssemblerARM32.cpp
index c4d5ad680c99e3cd19eb5d1e991a6f9423a2b2db..0132d709a20e992cf05c4d3d69d975105ba7c44f 100644
--- a/src/IceAssemblerARM32.cpp
+++ b/src/IceAssemblerARM32.cpp
@@ -175,22 +175,6 @@ RegARM32::GPRRegister getGPRReg(IValueT Shift, IValueT Value) {
   return decodeGPRRegister((Value >> Shift) & 0xF);
 }
 
-// Defines alternate layouts of instruction operands, should the (common)
-// default pattern not be used.
-enum OpEncoding {
-  // No alternate layout specified.
-  DefaultOpEncoding,
-  // Alternate encoding for ImmRegOffset, where the offset is divided by 4
-  // before encoding.
-  ImmRegOffsetDiv4,
-  // Alternate encoding 3 for memory operands (like in strb, strh, ldrb, and
-  // ldrh.
-  OpEncoding3,
-  // Alternate encoding for memory operands for ldrex and strex, which only
-  // actually expect a register.
-  OpEncodingMemEx
-};
-
 IValueT getEncodedGPRegNum(const Variable *Var) {
   assert(Var->hasReg());
   int32_t Reg = Var->getRegNum();
@@ -215,6 +199,23 @@ IValueT getYInRegYXXXX(IValueT RegYXXXX) { return RegYXXXX >> 4; }
 
 IValueT getXXXXInRegYXXXX(IValueT RegYXXXX) { return RegYXXXX & 0x0f; }
 
+// Defines layouts of an operand representing a (register) memory address,
+// possibly modified by an immediate value.
+enum EncodedImmAddress {
+  // Address modified by a rotated immediate 8-bit value.
+  RotatedImm8Address,
+  // Alternate encoding for RotatedImm8Address, where the offset is divided by 4

[Jim Stichnoth, 2016/01/25 20:37:50]

Optional: Consider adding a blank line after each enum value, to make it easier
to associate the comment with the correct enum value.

[Karl, 2016/01/25 23:59:16]

Added blank lines. Did same for following enum EncodedOperand.

+  // before encoding.
+  RotatedImm8Div4Address,
+  // Address modified by an immediate 12-bit value.
+  Imm12Address,
+  // Alternate encoding 3, for an address modified by a rotated immediate 8-bit
+  // value.
+  RotatedImm8Enc3Address,
+  // Encoding where no immediate offset is used.
+  NoImmOffsetAddress
+};
+
 // The way an operand is encoded into a sequence of bits in functions
 // encodeOperand and encodeAddress below.
 enum EncodedOperand {
@@ -226,24 +227,29 @@ enum EncodedOperand {
   // value.
   EncodedAsRotatedImm8,
   // EncodedAsImmRegOffset is a memory operand that can take three forms, based
-  // on OpEncoding:
+  // on type EncodedImmAddress:
   //
-  // ***** DefaultOpEncoding *****
+  // ***** RotatedImm8Address *****
   //
   // Value=0000000pu0w0nnnn0000iiiiiiiiiiii where nnnn is the base register Rn,
   // p=1 if pre-indexed addressing, u=1 if offset positive, w=1 if writeback to
   // Rn should be used, and iiiiiiiiiiii defines the rotated Imm8 value.
   //
-  // ***** OpEncoding3 *****
+  // ***** RotatedImm8Div4Address *****
   //
   // Value=00000000pu0w0nnnn0000iiii0000jjjj where nnnn=Rn, iiiijjjj=Imm8, p=1
   // if pre-indexed addressing, u=1 if offset positive, and w=1 if writeback to
   // Rn.
   //
-  // ***** OpEncodingMemEx *****
+  // ***** Imm12Address *****
   //
-  // Value=00000000U000nnnn00000000xxxxxxxx where nnnn=Rn, xxxxxxxx=abs(Offset),
-  // and U=1 Offset>=0.
+  // Value=0000000pu0w0nnnn0000iiiiiiiiiiii where nnnn is the base register Rn,
+  // p=1 if pre-indexed addressing, u=1 if offset positive, w=1 if writeback to
+  // Rn should be used, and iiiiiiiiiiii defines the immediate 12-bit value.
+  //
+  // ***** NoImmOffsetAddress *****
+  //
+  // Value=000000001000nnnn0000000000000000 where nnnn=Rn.
   EncodedAsImmRegOffset,
   // Value=0000000pu0w00nnnnttttiiiiiss0mmmm where nnnn is the base register Rn,
   // mmmm is the index register Rm, iiiii is the shift amount, ss is the shift
@@ -356,14 +362,16 @@ EncodedOperand encodeOperand(const Operand *Opnd, IValueT &Value,
   return CantEncode;
 }
 
-IValueT encodeImmRegOffset(IValueT Reg, IOffsetT Offset,
-                           OperandARM32Mem::AddrMode Mode,
-                           IValueT OffsetShift = 0) {
+inline IValueT encodeImmRegOffset(IValueT Reg, IOffsetT Offset,

[Jim Stichnoth, 2016/01/25 20:37:50]

Why inline here?  This seems inconsistent with the other functions here.

[Karl, 2016/01/25 23:59:16]

Removed.

+                                  OperandARM32Mem::AddrMode Mode,
+                                  IOffsetT MaxOffset, IValueT OffsetShift) {
   IValueT Value = Mode | (Reg << kRnShift);
   if (Offset < 0) {
     Offset = -Offset;
     Value ^= U; // Flip U to adjust sign.
   }
+  assert(Offset <= MaxOffset);
+  (void)MaxOffset;
   return Value | (Offset >> OffsetShift);
 }
 
@@ -380,23 +388,33 @@ IValueT encodeImmRegOffsetEnc3(IValueT Rn, IOffsetT Imm8,
   return Value;
 }
 
-IValueT encodeImmRegOffset(OpEncoding AddressEncoding, IValueT Reg,
+IValueT encodeImmRegOffset(EncodedImmAddress ImmEncoding, IValueT Reg,
                            IOffsetT Offset, OperandARM32Mem::AddrMode Mode) {
-  switch (AddressEncoding) {
-  case DefaultOpEncoding:
-    return encodeImmRegOffset(Reg, Offset, Mode);
-  case ImmRegOffsetDiv4: {
+  switch (ImmEncoding) {
+  case RotatedImm8Address: {
+    constexpr IOffsetT MaxOffset = (1 << 8) - 1;
+    constexpr IValueT NoRightShift = 0;
+    return encodeImmRegOffset(Reg, Offset, Mode, MaxOffset, NoRightShift);
+  }
+  case RotatedImm8Div4Address: {
     assert((Offset & 0x3) == 0);
+    constexpr IOffsetT MaxOffset = (1 << 8) - 1;
     constexpr IValueT RightShift2 = 2;
-    return encodeImmRegOffset(Reg, Offset, Mode, RightShift2);
+    return encodeImmRegOffset(Reg, Offset, Mode, MaxOffset, RightShift2);
+  }
+  case Imm12Address: {
+    constexpr IOffsetT MaxOffset = (1 << 12) - 1;
+    constexpr IValueT NoRightShift = 0;
+    return encodeImmRegOffset(Reg, Offset, Mode, MaxOffset, NoRightShift);
   }
-  case OpEncoding3:
+  case RotatedImm8Enc3Address:
     return encodeImmRegOffsetEnc3(Reg, Offset, Mode);
-  case OpEncodingMemEx:
+  case NoImmOffsetAddress: {
     assert(Offset == 0);
     assert(Mode == OperandARM32Mem::Offset);
     return Reg << kRnShift;
   }
+  }
   llvm_unreachable("(silence g++ warning)");
 }
 
@@ -404,7 +422,7 @@ IValueT encodeImmRegOffset(OpEncoding AddressEncoding, IValueT Reg,
 // on how ARM represents the address. Returns how the value was encoded.
 EncodedOperand encodeAddress(const Operand *Opnd, IValueT &Value,
                              const AssemblerARM32::TargetInfo &TInfo,
-                             OpEncoding AddressEncoding = DefaultOpEncoding) {
+                             EncodedImmAddress ImmEncoding) {
   Value = 0; // Make sure initialized.
   if (const auto *Var = llvm::dyn_cast<Variable>(Opnd)) {
     // Should be a stack variable, with an offset.
@@ -416,7 +434,7 @@ EncodedOperand encodeAddress(const Operand *Opnd, IValueT &Value,
     int32_t BaseRegNum = Var->getBaseRegNum();
     if (BaseRegNum == Variable::NoRegister)
       BaseRegNum = TInfo.FrameOrStackReg;
-    Value = encodeImmRegOffset(AddressEncoding, BaseRegNum, Offset,
+    Value = encodeImmRegOffset(ImmEncoding, BaseRegNum, Offset,
                                OperandARM32Mem::Offset);
     return EncodedAsImmRegOffset;
   }
@@ -436,7 +454,7 @@ EncodedOperand encodeAddress(const Operand *Opnd, IValueT &Value,
     }
     // Encoded as immediate register offset.
     ConstantInteger32 *Offset = Mem->getOffset();
-    Value = encodeImmRegOffset(AddressEncoding, Rn, Offset->getValue(),
+    Value = encodeImmRegOffset(ImmEncoding, Rn, Offset->getValue(),
                                Mem->getAddrMode());
     return EncodedAsImmRegOffset;
   }
@@ -802,7 +820,7 @@ void AssemblerARM32::emitMemOp(CondARM32::Cond Cond, bool IsLoad, bool IsByte,
                                IValueT Rt, const Operand *OpAddress,
                                const TargetInfo &TInfo, const char *InstName) {
   IValueT Address;
-  switch (encodeAddress(OpAddress, Address, TInfo)) {
+  switch (encodeAddress(OpAddress, Address, TInfo, Imm12Address)) {
   default:
     llvm::report_fatal_error(std::string(InstName) +
                              ": Memory address not understood");
@@ -860,7 +878,7 @@ void AssemblerARM32::emitMemOpEnc3(CondARM32::Cond Cond, IValueT Opcode,
                                    const TargetInfo &TInfo,
                                    const char *InstName) {
   IValueT Address;
-  switch (encodeAddress(OpAddress, Address, TInfo, OpEncoding3)) {
+  switch (encodeAddress(OpAddress, Address, TInfo, RotatedImm8Enc3Address)) {
   default:
     llvm::report_fatal_error(std::string(InstName) +
                              ": Memory address not understood");
@@ -1356,7 +1374,7 @@ void AssemblerARM32::emitMemExOp(CondARM32::Cond Cond, Type Ty, bool IsLoad,
     MemExOpcode |= B1;
   }
   IValueT AddressRn;
-  if (encodeAddress(OpAddress, AddressRn, TInfo, OpEncodingMemEx) !=
+  if (encodeAddress(OpAddress, AddressRn, TInfo, NoImmOffsetAddress) !=
       EncodedAsImmRegOffset)
     llvm::report_fatal_error(std::string(InstName) +
                              ": Can't extract Rn from address");
@@ -1730,8 +1748,11 @@ void AssemblerARM32::pop(const Variable *OpRt, CondARM32::Cond Cond) {
   // Same as load instruction.
   constexpr bool IsLoad = true;
   constexpr bool IsByte = false;
-  IValueT Address = encodeImmRegOffset(RegARM32::Encoded_Reg_sp, kWordSize,
-                                       OperandARM32Mem::PostIndex);
+  constexpr IOffsetT MaxOffset = (1 << 8) - 1;
+  constexpr IValueT NoShiftRight = 0;
+  IValueT Address =
+      encodeImmRegOffset(RegARM32::Encoded_Reg_sp, kWordSize,
+                         OperandARM32Mem::PostIndex, MaxOffset, NoShiftRight);
   emitMemOp(Cond, kInstTypeMemImmediate, IsLoad, IsByte, Rt, Address);
 }
 
@@ -1756,8 +1777,11 @@ void AssemblerARM32::push(const Operand *OpRt, CondARM32::Cond Cond) {
   // Same as store instruction.
   constexpr bool isLoad = false;
   constexpr bool isByte = false;
-  IValueT Address = encodeImmRegOffset(RegARM32::Encoded_Reg_sp, -kWordSize,
-                                       OperandARM32Mem::PreIndex);
+  constexpr IOffsetT MaxOffset = (1 << 8) - 1;
+  constexpr IValueT NoShiftRight = 0;
+  IValueT Address =
+      encodeImmRegOffset(RegARM32::Encoded_Reg_sp, -kWordSize,
+                         OperandARM32Mem::PreIndex, MaxOffset, NoShiftRight);
   emitMemOp(Cond, kInstTypeMemImmediate, isLoad, isByte, Rt, Address);
 }
 
@@ -2322,7 +2346,7 @@ void AssemblerARM32::vldrd(const Operand *OpDd, const Operand *OpAddress,
   assert(CondARM32::isDefined(Cond));
   IValueT Address;
   EncodedOperand AddressEncoding =
-      encodeAddress(OpAddress, Address, TInfo, ImmRegOffsetDiv4);
+      encodeAddress(OpAddress, Address, TInfo, RotatedImm8Div4Address);
   (void)AddressEncoding;
   assert(AddressEncoding == EncodedAsImmRegOffset);
   IValueT Encoding = B27 | B26 | B24 | B20 | B11 | B9 | B8 |
@@ -2344,7 +2368,7 @@ void AssemblerARM32::vldrs(const Operand *OpSd, const Operand *OpAddress,
   assert(CondARM32::isDefined(Cond));
   IValueT Address;
   EncodedOperand AddressEncoding =
-      encodeAddress(OpAddress, Address, TInfo, ImmRegOffsetDiv4);
+      encodeAddress(OpAddress, Address, TInfo, RotatedImm8Div4Address);
   (void)AddressEncoding;
   assert(AddressEncoding == EncodedAsImmRegOffset);
   IValueT Encoding = B27 | B26 | B24 | B20 | B11 | B9 |
@@ -2429,7 +2453,7 @@ void AssemblerARM32::vstrd(const Operand *OpDd, const Operand *OpAddress,
   assert(CondARM32::isDefined(Cond));
   IValueT Address;
   IValueT AddressEncoding =
-      encodeAddress(OpAddress, Address, TInfo, ImmRegOffsetDiv4);
+      encodeAddress(OpAddress, Address, TInfo, RotatedImm8Div4Address);
   (void)AddressEncoding;
   assert(AddressEncoding == EncodedAsImmRegOffset);
   IValueT Encoding = B27 | B26 | B24 | B11 | B9 | B8 |
@@ -2451,7 +2475,7 @@ void AssemblerARM32::vstrs(const Operand *OpSd, const Operand *OpAddress,
   assert(CondARM32::isDefined(Cond));
   IValueT Address;
   IValueT AddressEncoding =
-      encodeAddress(OpAddress, Address, TInfo, ImmRegOffsetDiv4);
+      encodeAddress(OpAddress, Address, TInfo, RotatedImm8Div4Address);
   (void)AddressEncoding;
   assert(AddressEncoding == EncodedAsImmRegOffset);
   IValueT Encoding =