Add various forms of LDREX/STREX to ARM integrated assembler.

src/IceAssemblerARM32.cpp

Index: src/IceAssemblerARM32.cpp
diff --git a/src/IceAssemblerARM32.cpp b/src/IceAssemblerARM32.cpp
index 2ef7e2f8a0174071af5b8e2c86007761c2e86eb5..8e7c5cd87b312f7d847915914f9d86c234349bed 100644
--- a/src/IceAssemblerARM32.cpp
+++ b/src/IceAssemblerARM32.cpp
@@ -41,6 +41,10 @@ static constexpr IValueT B4 = 1 << 4;
 static constexpr IValueT B5 = 1 << 5;
 static constexpr IValueT B6 = 1 << 6;
 static constexpr IValueT B7 = 1 << 7;
+static constexpr IValueT B8 = 1 << 8;
+static constexpr IValueT B9 = 1 << 9;
+static constexpr IValueT B10 = 1 << 10;
+static constexpr IValueT B11 = 1 << 11;
 static constexpr IValueT B12 = 1 << 12;
 static constexpr IValueT B13 = 1 << 13;
 static constexpr IValueT B14 = 1 << 14;
@@ -96,6 +100,9 @@ static constexpr IValueT kImm12Shift = 0;
 // Rotation instructions (uxtb etc.).
 static constexpr IValueT kRotationShift = 10;
 
+// MemEx instructions.
+static constexpr IValueT kMemExOpcodeShift = 20;
+
 // Div instruction register field encodings.
 static constexpr IValueT kDivRdShift = 16;
 static constexpr IValueT kDivRmShift = 8;
@@ -173,6 +180,25 @@ 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 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 getVarRegNum(const Variable *Var) {

[Jim Stichnoth, 2015/12/11 17:24:57]

This function name should be changed, to reflect that fact that it is returning
an encoded register number.

Maybe getEncodedReg or getEncodedRegNum ?

[Karl, 2015/12/11 18:05:08]

Changed to getEncodedGPRegNum.

+  int32_t Reg = Var->getRegNum();
+  return llvm::isa<Variable64On32>(Var) ? RegARM32::getI64PairFirstGPRNum(Reg)
+                                        : RegARM32::getEncodedGPR(Reg);
+}
+
 // The way an operand is encoded into a sequence of bits in functions
 // encodeOperand and encodeAddress below.
 enum EncodedOperand {
@@ -183,14 +209,25 @@ enum EncodedOperand {
   // Value=rrrriiiiiiii where rrrr is the rotation, and iiiiiiii is the imm8
   // value.
   EncodedAsRotatedImm8,
+  // EncodedAsImmRegOffset is a memory operand that can take three forms, based
+  // on OpEncoding:
+  //
+  // case DefaultOpEncoding:

[Jim Stichnoth, 2015/12/11 17:24:57]

This style of comment made me think you had some stray commented code lying
around.  What do you think of this style:

// *** DefaultOpEncoding ***
// Value=...
//
// **** OpEncoding3 ***
// Value=...

or something along those lines?

[Karl, 2015/12/11 18:05:08]

Ok. Done.

+  //
   // 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.
-  EncodedAsImmRegOffset,
+  //
+  // case OpEncoding3:
+  //
   // 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.
-  EncodedAsImmRegOffsetEnc3,
+  //
+  // case OpEncodingMemEx:
+  //
+  // Value=000000000000nnnn0000000000000000 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
   // kind, p=1 if pre-indexed addressing, u=1 if offset positive, and w=1 if
@@ -234,7 +271,7 @@ EncodedOperand encodeOperand(const Operand *Opnd, IValueT &Value) {
   Value = 0; // Make sure initialized.
   if (const auto *Var = llvm::dyn_cast<Variable>(Opnd)) {
     if (Var->hasReg()) {
-      Value = Var->getRegNum();
+      Value = getVarRegNum(Var);
       return EncodedAsRegister;
     }
     return CantEncode;
@@ -296,14 +333,19 @@ IValueT encodeImmRegOffsetEnc3(IValueT Rn, IOffsetT Imm8,
   return Value;
 }
 
-// Defines alternate layouts of instruction operands, should the (common)
-// default pattern not be used.
-enum OpEncoding {
-  // No alternate layout specified.
-  DefaultOpEncoding,
-  // Alternate encoding 3.
-  OpEncoding3
-};
+IValueT encodeImmRegOffset(OpEncoding AddressEncoding, IValueT Reg,
+                           IOffsetT Offset, OperandARM32Mem::AddrMode Mode) {
+  switch (AddressEncoding) {
+  case DefaultOpEncoding:
+    return encodeImmRegOffset(Reg, Offset, Mode);
+  case OpEncoding3:
+    return encodeImmRegOffsetEnc3(Reg, Offset, Mode);
+  case OpEncodingMemEx:
+    assert(Offset == 0);
+    assert(Mode == OperandARM32Mem::Offset);
+    return Reg << kRnShift;
+  }
+}
 
 // Encodes memory address Opnd, and encodes that information into Value, based
 // on how ARM represents the address. Returns how the value was encoded.
@@ -321,34 +363,29 @@ EncodedOperand encodeAddress(const Operand *Opnd, IValueT &Value,
     int32_t BaseRegNum = Var->getBaseRegNum();
     if (BaseRegNum == Variable::NoRegister)
       BaseRegNum = TInfo.FrameOrStackReg;
-    Value = encodeImmRegOffset(BaseRegNum, Offset, OperandARM32Mem::Offset);
+    Value = encodeImmRegOffset(AddressEncoding, BaseRegNum, Offset,
+                               OperandARM32Mem::Offset);
     return EncodedAsImmRegOffset;
   }
   if (const auto *Mem = llvm::dyn_cast<OperandARM32Mem>(Opnd)) {
     Variable *Var = Mem->getBase();
     if (!Var->hasReg())
       return CantEncode;
-    IValueT Rn = Var->getRegNum();
+    IValueT Rn = getVarRegNum(Var);
     if (Mem->isRegReg()) {
       const Variable *Index = Mem->getIndex();
       if (Var == nullptr)
         return CantEncode;
       Value = (Rn << kRnShift) | Mem->getAddrMode() |
-              encodeShiftRotateImm5(Index->getRegNum(), Mem->getShiftOp(),
+              encodeShiftRotateImm5(getVarRegNum(Index), Mem->getShiftOp(),
                                     Mem->getShiftAmt());
       return EncodedAsShiftRotateImm5;
     }
     // Encoded as immediate register offset.
     ConstantInteger32 *Offset = Mem->getOffset();
-    switch (AddressEncoding) {
-    case DefaultOpEncoding:
-      Value = encodeImmRegOffset(Rn, Offset->getValue(), Mem->getAddrMode());
-      return EncodedAsImmRegOffset;
-    case OpEncoding3:
-      Value =
-          encodeImmRegOffsetEnc3(Rn, Offset->getValue(), Mem->getAddrMode());
-      return EncodedAsImmRegOffsetEnc3;
-    }
+    Value = encodeImmRegOffset(AddressEncoding, Rn, Offset->getValue(),
+                               Mem->getAddrMode());
+    return EncodedAsImmRegOffset;
   }
   return CantEncode;
 }
@@ -768,7 +805,7 @@ void AssemblerARM32::emitMemOpEnc3(CondARM32::Cond Cond, IValueT Opcode,
   default:
     llvm::report_fatal_error(std::string(InstName) +
                              ": Memory address not understood");
-  case EncodedAsImmRegOffsetEnc3: {
+  case EncodedAsImmRegOffset: {
     // XXXH (immediate)
     //   xxxh<c> <Rt>, [<Rn>{, #+-<Imm8>}]
     //   xxxh<c> <Rt>, [<Rn>, #+/-<Imm8>]
@@ -1139,7 +1176,7 @@ void AssemblerARM32::ldr(const Operand *OpRt, const Operand *OpAddress,
   // case.
   default:
     llvm::report_fatal_error(std::string("ldr : Type ") + typeString(Ty) +
-                             " not implementable\n");
+                             " not implementable");
   case 0: {
     // Handles i1 and i8 loads.
     //
@@ -1202,6 +1239,74 @@ void AssemblerARM32::ldr(const Operand *OpRt, const Operand *OpAddress,
   }
 }
 
+void AssemblerARM32::emitMemExOp(CondARM32::Cond Cond, Type Ty, bool IsLoad,
+                                 const Operand *OpRd, IValueT Rt,
+                                 const Operand *OpAddress,
+                                 const TargetInfo &TInfo,
+                                 const char *InstName) {
+  IValueT Rd = encodeRegister(OpRd, "Rd", InstName);
+  IValueT MemExOpcode = IsLoad ? B0 : 0;
+  switch (typeWidthInBytesLog2(Ty)) {

[John, 2015/12/11 16:59:09]

Why did you switch on the type width? can you switch (Ty) instead?

[Karl, 2015/12/11 18:05:08]

I did it this way for the following reasons:

1) I wanted only one source of type width (for each type), so that if it
changes, no other code needs fixing.

2) Using the log2 form, there is a simple contiguous switch selection (allowing
possible optimization of the switch).

[John, 2015/12/11 18:10:46]

IceType_i[8|16|32|64] are declared sequentially in the enum, so the compiler
should be able to optimize this.

Please, switch on Ty. It will make the code explicit (i.e., ldrex/strex operate
on integer types.) anything other than i8, i16, i32, or i64 here is a mistake
and should be reported.

[Karl, 2015/12/11 21:00:12]

OK, I'll change it, but you are slightly incorrect. type i1 uses the "b" form
also.

We should also be consistent, so I'm fixing the other cases where
typeWidthInBits is used.

+  default:
+    llvm::report_fatal_error(std::string(InstName) + ": Type " +
+                             typeString(Ty) + " not implementable");
+  case 0:
+    MemExOpcode |= B2;
+    break;
+  case 1:
+    MemExOpcode |= B2 | B1;
+    break;
+  case 2:
+    break;
+  case 3:
+    MemExOpcode |= B1;
+  }
+  // emitMemExOp(Cond, MemExOpcode, Rd, Rt, OpAddress, TInfo, InstName);

[Jim Stichnoth, 2015/12/11 17:24:57]

Remove this comment?

[Karl, 2015/12/11 18:05:08]

Really? I like useless comments. Removing...

+  IValueT AddressRn;
+  if (encodeAddress(OpAddress, AddressRn, TInfo, OpEncodingMemEx) !=
+      EncodedAsImmRegOffset)
+    llvm::report_fatal_error(std::string(InstName) +
+                             ": Can't extract Rn from address");
+  assert(Utils::IsAbsoluteUint(3, MemExOpcode));
+  verifyRegDefined(Rd, "Rd", InstName);
+  verifyRegDefined(Rt, "Rt", InstName);
+  verifyCondDefined(Cond, InstName);
+  AssemblerBuffer::EnsureCapacity ensured(&Buffer);
+  IValueT Encoding = (Cond << kConditionShift) | B24 | B23 | B11 | B10 | B9 |
+                     B8 | B7 | B4 | (MemExOpcode << kMemExOpcodeShift) |
+                     AddressRn | (Rd << kRdShift) | (Rt << kRmShift);
+  emitInst(Encoding);
+  return;
+}
+
+void AssemblerARM32::ldrex(const Operand *OpRt, const Operand *OpAddress,
+                           CondARM32::Cond Cond, const TargetInfo &TInfo) {
+  // LDREXB - ARM section A8.8.76, encoding A1:
+  //   ldrexb<c> <Rt>, [<Rn>]
+  //
+  // cccc00011101nnnntttt111110011111 where cccc=Cond, tttt=Rt, and nnnn=Rn.
+  //
+  // LDREXH - ARM section A8.8.78, encoding A1:
+  //   ldrexh<c> <Rt>, [<Rn>]
+  //
+  // cccc00011111nnnntttt111110011111 where cccc=Cond, tttt=Rt, and nnnn=Rn.
+  //
+  // LDREX - ARM section A8.8.75, encoding A1:
+  //   ldrex<c> <Rt>, [<Rn>]
+  //
+  // cccc00011001nnnntttt111110011111 where cccc=Cond, tttt=Rt, and nnnn=Rn.
+  //
+  // LDREXD - ARM section A8.
+  //   ldrexd<c> <Rt>, [<Rn>]
+  //
+  // cccc00011001nnnntttt111110011111 where cccc=Cond, tttt=Rt, and nnnn=Rn.
+  constexpr const char *LdrexName = "ldrex";
+  const Type Ty = OpRt->getType();
+  constexpr bool IsLoad = true;
+  constexpr IValueT Rm = RegARM32::Encoded_Reg_pc;
+  emitMemExOp(Cond, Ty, IsLoad, OpRt, Rm, OpAddress, TInfo, LdrexName);
+}
+
 void AssemblerARM32::emitShift(const CondARM32::Cond Cond,
                                const OperandARM32::ShiftKind Shift,
                                const Operand *OpRd, const Operand *OpRm,
@@ -1471,6 +1576,41 @@ void AssemblerARM32::str(const Operand *OpRt, const Operand *OpAddress,
   }
 }
 
+void AssemblerARM32::strex(const Operand *OpRd, const Operand *OpRt,
+                           const Operand *OpAddress, CondARM32::Cond Cond,
+                           const TargetInfo &TInfo) {
+  // STREXB - ARM section A8.8.213, encoding A1:
+  //   strexb<c> <Rd>, <Rt>, [<Rn>]
+  //
+  // cccc00011100nnnndddd11111001tttt where cccc=Cond, dddd=Rd, tttt=Rt, and
+  // nnnn=Rn.
+  //
+  // STREXH - ARM section A8.8.215, encoding A1:
+  //   strexh<c> <Rd>, <Rt>, [<Rn>]
+  //
+  // cccc00011110nnnndddd11111001tttt where cccc=Cond, dddd=Rd, tttt=Rt, and
+  // nnnn=Rn.
+  //
+  // STREX - ARM section A8.8.212, encoding A1:
+  //   strex<c> <Rd>, <Rt>, [<Rn>]
+  //
+  // cccc00011000nnnndddd11111001tttt where cccc=Cond, dddd=Rd, tttt=Rt, and
+  // nnnn=Rn.
+  //
+  // STREXD - ARM section A8.8.214, encoding A1:
+  //   strexd<c> <Rd>, <Rt>, [<Rn>]
+  //
+  // cccc00011010nnnndddd11111001tttt where cccc=Cond, dddd=Rd, tttt=Rt, and
+  // nnnn=Rn.
+  constexpr const char *StrexName = "strex";
+  // Note: Rt uses Rm shift in encoding.
+  IValueT Rt = encodeRegister(OpRt, "Rt", StrexName);
+  const Type Ty = OpRt->getType();
+  constexpr bool IsLoad = true;
+  emitMemExOp(Cond, Ty, !IsLoad, OpRd, Rt, OpAddress, TInfo, StrexName);
+  return;

[Jim Stichnoth, 2015/12/11 17:24:57]

Remove this explicit return.

[Karl, 2015/12/11 18:05:08]

Done.

+}
+
 void AssemblerARM32::orr(const Operand *OpRd, const Operand *OpRn,
                          const Operand *OpSrc1, bool SetFlags,
                          CondARM32::Cond Cond) {