Fix vldrs/vstrs handling of immediate offsets in ARM.

src/IceAssemblerARM32.cpp

 //===- subzero/src/IceAssemblerARM32.cpp - Assembler for ARM32 --*- C++ -*-===//
 //
 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 //
 // Modified by the Subzero authors.
 //
 //===----------------------------------------------------------------------===//
 //
@@ skipping 338 matching lines @@
   if (const auto *ShImm = llvm::dyn_cast<OperandARM32ShAmtImm>(Opnd)) {
     const IValueT Immed5 = ShImm->getShAmtImm();
     assert(Immed5 < (1 << kShiftImmBits));
     Value = (Immed5 << kShiftImmShift);
     return EncodedAsShiftImm5;
   }
   return CantEncode;
 }
 
 IValueT encodeImmRegOffset(IValueT Reg, IOffsetT Offset,
-                           OperandARM32Mem::AddrMode Mode) {
+                           OperandARM32Mem::AddrMode Mode,
+                           IValueT OffsetShift = 0) {
   IValueT Value = Mode | (Reg << kRnShift);
   if (Offset < 0) {
-    Value = (Value ^ U) | -Offset; // Flip U to adjust sign.
-  } else {
-    Value |= Offset;
+    Offset = -Offset;

[Jim Stichnoth, 2016/01/22 22:47:15]

I assume this is probably not possible, but it sets off a boundary-case trigger
for me - what happens when Offset == std::numeric_limits<IValueT>::lowest()? 
(i.e. MININT)

[John, 2016/01/22 23:33:25]

arm32 immediates are small (usually 8-bits, sometimes 12-, rarely 24-bits) so
this should not be a problem.

[Karl, 2016/01/25 19:03:26]

Fixing with CL https://codereview.chromium.org/1630863002

+    Value ^= U; // Flip U to adjust sign.
   }
-  return Value;
+  return Value | (Offset >> OffsetShift);
 }
 
 // Encodes immediate register offset using encoding 3.
 IValueT encodeImmRegOffsetEnc3(IValueT Rn, IOffsetT Imm8,
                                OperandARM32Mem::AddrMode Mode) {
   IValueT Value = Mode | (Rn << kRnShift);
   if (Imm8 < 0) {
     Imm8 = -Imm8;
     Value = (Value ^ U);
   }
   assert(Imm8 < (1 << 8));
   Value = Value | B22 | ((Imm8 & 0xf0) << 4) | (Imm8 & 0x0f);
   return Value;
 }
 
 IValueT encodeImmRegOffset(OpEncoding AddressEncoding, IValueT Reg,
                            IOffsetT Offset, OperandARM32Mem::AddrMode Mode) {
   switch (AddressEncoding) {
   case DefaultOpEncoding:
     return encodeImmRegOffset(Reg, Offset, Mode);
-  case ImmRegOffsetDiv4:
+  case ImmRegOffsetDiv4: {
     assert((Offset & 0x3) == 0);
-    return encodeImmRegOffset(Reg, Offset >> 2, Mode);
+    constexpr IValueT RightShift2 = 2;
+    return encodeImmRegOffset(Reg, Offset, Mode, RightShift2);
+  }
   case OpEncoding3:
     return encodeImmRegOffsetEnc3(Reg, Offset, Mode);
   case OpEncodingMemEx:
     assert(Offset == 0);
     assert(Mode == OperandARM32Mem::Offset);
     return Reg << kRnShift;
   }
   llvm_unreachable("(silence g++ warning)");
 }
 
@@ skipping 201 matching lines @@
   }
   return L;
 }
 
 IValueT AssemblerARM32::encodeBranchOffset(IOffsetT Offset, IValueT Inst) {
   // Adjust offset to the way ARM CPUs read PC.
   Offset -= kPCReadOffset;
 
   bool IsGoodOffset = canEncodeBranchOffset(Offset);
   assert(IsGoodOffset);
-  // Note: Following cast is for MINIMAL build.
   (void)IsGoodOffset;
 
   // Properly preserve only the bits supported in the instruction.
   Offset >>= 2;
   Offset &= kBranchOffsetMask;
   return (Inst & ~kBranchOffsetMask) | Offset;
 }
 
 // Pull out offset from branch Inst.
 IOffsetT AssemblerARM32::decodeBranchOffset(IValueT Inst) {
@@ skipping 1613 matching lines @@
   //   vldr<c> <Dd>, [<Rn>{, #+/-<imm>}]
   //
   // cccc1101UD01nnnndddd1011iiiiiiii where cccc=Cond, nnnn=Rn, Ddddd=Rd,
   // iiiiiiii=abs(Imm >> 2), and U=1 if Opcode>=0.
   constexpr const char *Vldrd = "vldrd";
   IValueT Dd = encodeDRegister(OpDd, "Dd", Vldrd);
   assert(CondARM32::isDefined(Cond));
   IValueT Address;
   EncodedOperand AddressEncoding =
       encodeAddress(OpAddress, Address, TInfo, ImmRegOffsetDiv4);
-  if (AddressEncoding != EncodedAsImmRegOffset)
-    // TODO(kschimpf) Fix this.
-    return setNeedsTextFixup();
+  (void)AddressEncoding;
+  assert(AddressEncoding == EncodedAsImmRegOffset);
   AssemblerBuffer::EnsureCapacity ensured(&Buffer);
   IValueT Encoding = B27 | B26 | B24 | B20 | B11 | B9 | B8 |
                      (encodeCondition(Cond) << kConditionShift) |
                      (getYInRegYXXXX(Dd) << 22) |
                      (getXXXXInRegYXXXX(Dd) << 12) | Address;
   emitInst(Encoding);
 }
 
 void AssemblerARM32::vldrs(const Operand *OpSd, const Operand *OpAddress,
                            CondARM32::Cond Cond, const TargetInfo &TInfo) {
   // VDLR - ARM section A8.8.333, encoding A2.
   //   vldr<c> <Sd>, [<Rn>{, #+/-<imm>]]
   //
   // cccc1101UD01nnnndddd1010iiiiiiii where cccc=Cond, nnnn=Rn, ddddD=Sd,
   // iiiiiiii=abs(Opcode), and U=1 if Opcode >= 0;
   constexpr const char *Vldrs = "vldrs";
   IValueT Sd = encodeSRegister(OpSd, "Sd", Vldrs);
   assert(CondARM32::isDefined(Cond));
   IValueT Address;
-  EncodedOperand AddressEncoding = encodeAddress(OpAddress, Address, TInfo);
+  EncodedOperand AddressEncoding =
+      encodeAddress(OpAddress, Address, TInfo, ImmRegOffsetDiv4);
   (void)AddressEncoding;
   assert(AddressEncoding == EncodedAsImmRegOffset);
   AssemblerBuffer::EnsureCapacity ensured(&Buffer);
   IValueT Encoding = B27 | B26 | B24 | B20 | B11 | B9 |
                      (encodeCondition(Cond) << kConditionShift) |
                      (getYInRegXXXXY(Sd) << 22) |
                      (getXXXXInRegXXXXY(Sd) << 12) | Address;
   emitInst(Encoding);
 }
 
@@ skipping 33 matching lines @@
   //   vstr<c> <Dd>, [<Rn>{, #+/-<Imm>}]
   //
   // cccc1101UD00nnnndddd1011iiiiiiii where cccc=Cond, nnnn=Rn, Ddddd=Rd,
   // iiiiiiii=abs(Imm >> 2), and U=1 if Imm>=0.
   constexpr const char *Vstrd = "vstrd";
   IValueT Dd = encodeDRegister(OpDd, "Dd", Vstrd);
   assert(CondARM32::isDefined(Cond));
   IValueT Address;
   IValueT AddressEncoding =
       encodeAddress(OpAddress, Address, TInfo, ImmRegOffsetDiv4);
-  if (AddressEncoding != EncodedAsImmRegOffset)
-    // TODO(kschimpf) Fix this.
-    return setNeedsTextFixup();
+  (void)AddressEncoding;
+  assert(AddressEncoding == EncodedAsImmRegOffset);
   AssemblerBuffer::EnsureCapacity ensured(&Buffer);
   IValueT Encoding = B27 | B26 | B24 | B11 | B9 | B8 |
                      (encodeCondition(Cond) << kConditionShift) |
                      (getYInRegYXXXX(Dd) << 22) |
                      (getXXXXInRegYXXXX(Dd) << 12) | Address;
   emitInst(Encoding);
 }
 
 void AssemblerARM32::vstrs(const Operand *OpSd, const Operand *OpAddress,
                            CondARM32::Cond Cond, const TargetInfo &TInfo) {
   // VSTR - ARM section A8.8.413, encoding A2:
   //   vstr<c> <Sd>, [<Rn>{, #+/-<imm>]]
   //
   // cccc1101UD01nnnndddd1010iiiiiiii where cccc=Cond, nnnn=Rn, ddddD=Sd,
   // iiiiiiii=abs(Opcode), and U=1 if Opcode >= 0;
   constexpr const char *Vstrs = "vstrs";
   IValueT Sd = encodeSRegister(OpSd, "Sd", Vstrs);
   assert(CondARM32::isDefined(Cond));
   IValueT Address;
-  if (encodeAddress(OpAddress, Address, TInfo) != EncodedAsImmRegOffset)
-    assert(false);
+  IValueT AddressEncoding =
+      encodeAddress(OpAddress, Address, TInfo, ImmRegOffsetDiv4);
+  (void)AddressEncoding;
+  assert(AddressEncoding == EncodedAsImmRegOffset);
   AssemblerBuffer::EnsureCapacity ensured(&Buffer);
   IValueT Encoding =
       B27 | B26 | B24 | B11 | B9 | (encodeCondition(Cond) << kConditionShift) |
       (getYInRegXXXXY(Sd) << 22) | (getXXXXInRegXXXXY(Sd) << 12) | Address;
   emitInst(Encoding);
 }
 
 void AssemblerARM32::vsubs(const Operand *OpSd, const Operand *OpSn,
                            const Operand *OpSm, CondARM32::Cond Cond) {
   // VSUB (floating-point) - ARM section A8.8.415, encoding A2:
@@ skipping 66 matching lines @@
   //
   // cccc11010D101101dddd1010iiiiiiii where cccc=Cond, ddddD=BaseReg, and
   // iiiiiiii=NumConsecRegs.
   constexpr IValueT VpushOpcode =
       B27 | B26 | B24 | B21 | B19 | B18 | B16 | B11 | B9;
   emitVStackOp(Cond, VpushOpcode, OpBaseReg, NumConsecRegs);
 }
 
 } // end of namespace ARM32
 } // end of namespace Ice