Clean up ARM assembly code loading and storing from/to a large offset.

runtime/vm/assembler_arm.cc

 // 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.
 
 #include "vm/globals.h"
 #if defined(TARGET_ARCH_ARM)
 
 #include "vm/assembler.h"
 #include "vm/simulator.h"
 #include "vm/runtime_entry.h"
 #include "vm/stub_code.h"
 
 namespace dart {
 
-// TODO(regis): Enable this flag after PrintStopMessage stub is implemented.
-DEFINE_FLAG(bool, print_stop_message, false, "Print stop message.");
+DEFINE_FLAG(bool, print_stop_message, true, "Print stop message.");
 
 
 bool CPUFeatures::integer_division_supported_ = false;
 #if defined(DEBUG)
 bool CPUFeatures::initialized_ = false;
 #endif
 
 
 bool CPUFeatures::integer_division_supported() {
   DEBUG_ASSERT(initialized_);
@@ skipping 1215 matching lines @@
   if (object.IsNull() ||
       object.IsSmi() ||
       (object.raw() == Bool::True().raw()) ||
       (object.raw() == Bool::False().raw())) {
     // This object is never relocated; do not use object pool.
     LoadImmediate(rd, reinterpret_cast<int32_t>(object.raw()));
     return;
   }
   const int32_t offset =
       Array::data_offset() + 4*AddObject(object) - kHeapObjectTag;
-  if (Address::CanHoldLoadOffset(kLoadWord, offset)) {
+  int32_t offset_mask;
+  if (Address::CanHoldLoadOffset(kLoadWord, offset, &offset_mask)) {
     ldr(rd, Address(PP, offset));
   } else {
-    int32_t offset12_hi = offset & ~kOffset12Mask;  // signed
-    uint32_t offset12_lo = offset & kOffset12Mask;  // unsigned
-    AddImmediate(rd, PP, offset12_hi);
-    ldr(rd, Address(rd, offset12_lo));
+    int32_t offset_hi = offset & ~offset_mask;  // signed
+    uint32_t offset_lo = offset & offset_mask;  // unsigned

[zra, 2013/03/06 16:41:27]

ASSERT(CanHoldLoadOffset(offset_lo))

[regis, 2013/03/06 18:42:00]

Actually, each instruction already asserts that the offset is in range. So
instead of adding extra asserts here, I prefer to remove those in LoadFromOffset
and friends.

+    AddImmediate(rd, PP, offset_hi);
+    ldr(rd, Address(rd, offset_lo));
   }
 }
 
 
 void Assembler::Bind(Label* label) {
   ASSERT(!label->IsBound());
   int bound_pc = buffer_.Size();
   while (label->IsLinked()) {
     int32_t position = label->Position();
     int32_t next = buffer_.Load<int32_t>(position);
     int32_t encoded = Assembler::EncodeBranchOffset(bound_pc - position, next);
     buffer_.Store<int32_t>(position, encoded);
     label->position_ = Assembler::DecodeBranchOffset(next);
   }
   label->BindTo(bound_pc);
 }
 
 
-bool Address::CanHoldLoadOffset(LoadOperandType type, int offset) {
+bool Address::CanHoldLoadOffset(LoadOperandType type,

[zra, 2013/03/06 03:30:28]

I just want to double-check this and CanHoldStoreOffset against the spec first
thing tomorrow morning, but otherwise I think these changes are good.

[regis, 2013/03/06 18:42:00]

OK. I'll revisit if you find a problem.

+                                int32_t offset,
+                                int32_t* offset_mask) {
   switch (type) {
     case kLoadSignedByte:
     case kLoadSignedHalfword:
     case kLoadUnsignedHalfword:
-    case kLoadWordPair:
+    case kLoadWordPair: {
+      *offset_mask = 0xff;
       return Utils::IsAbsoluteUint(8, offset);  // Addressing mode 3.
+    }
     case kLoadUnsignedByte:
-    case kLoadWord:
+    case kLoadWord: {
+      *offset_mask = 0xfff;
       return Utils::IsAbsoluteUint(12, offset);  // Addressing mode 2.
+    }
     case kLoadSWord:
-    case kLoadDWord:
+    case kLoadDWord: {
+      *offset_mask = 0x3ff;
       return Utils::IsAbsoluteUint(10, offset);  // VFP addressing mode.
-    default:
+    }
+    default: {
       UNREACHABLE();
       return false;
+    }
   }
 }
 
 
-bool Address::CanHoldStoreOffset(StoreOperandType type, int offset) {
+bool Address::CanHoldStoreOffset(StoreOperandType type,
+                                 int32_t offset,
+                                 int32_t* offset_mask) {
   switch (type) {
     case kStoreHalfword:
-    case kStoreWordPair:
+    case kStoreWordPair: {
+      *offset_mask = 0xff;
       return Utils::IsAbsoluteUint(8, offset);  // Addressing mode 3.
+    }
     case kStoreByte:
-    case kStoreWord:
+    case kStoreWord: {
+      *offset_mask = 0xfff;
       return Utils::IsAbsoluteUint(12, offset);  // Addressing mode 2.
+    }
     case kStoreSWord:
-    case kStoreDWord:
+    case kStoreDWord: {
+      *offset_mask = 0x3ff;
       return Utils::IsAbsoluteUint(10, offset);  // VFP addressing mode.
-    default:
+    }
+    default: {
       UNREACHABLE();
       return false;
+    }
   }
 }
 
 
 void Assembler::Push(Register rd, Condition cond) {
   str(rd, Address(SP, -kWordSize, Address::PreIndex), cond);
 }
 
 
 void Assembler::Pop(Register rd, Condition cond) {
@@ skipping 65 matching lines @@
 }
 
 
 void Assembler::BranchLinkPatchable(const ExternalLabel* label) {
   // Make sure that class CallPattern is able to patch the label referred
   // to by this code sequence.
   // For added code robustness, use 'blx lr' in a patchable sequence and
   // use 'blx ip' in a non-patchable sequence (see other BranchLink flavors).
   const int32_t offset =
       Array::data_offset() + 4*AddExternalLabel(label) - kHeapObjectTag;
-  if (Address::CanHoldLoadOffset(kLoadWord, offset)) {
+  int32_t offset_mask;
+  if (Address::CanHoldLoadOffset(kLoadWord, offset, &offset_mask)) {
     ldr(LR, Address(PP, offset));
   } else {
-    int32_t offset12_hi = offset & ~kOffset12Mask;  // signed
-    uint32_t offset12_lo = offset & kOffset12Mask;  // unsigned
-    // Inline a simplified version of AddImmediate(LR, CP, offset12_hi).
+    int32_t offset_hi = offset & ~offset_mask;  // signed
+    uint32_t offset_lo = offset & offset_mask;  // unsigned
+    // Inline a simplified version of AddImmediate(LR, CP, offset_hi).
     ShifterOperand shifter_op;
-    if (ShifterOperand::CanHold(offset12_hi, &shifter_op)) {
+    if (ShifterOperand::CanHold(offset_hi, &shifter_op)) {
       add(LR, PP, shifter_op);
     } else {
-      movw(LR, Utils::Low16Bits(offset12_hi));
-      const uint16_t value_high = Utils::High16Bits(offset12_hi);
+      movw(LR, Utils::Low16Bits(offset_hi));
+      const uint16_t value_high = Utils::High16Bits(offset_hi);
       if (value_high != 0) {
         movt(LR, value_high);
       }
       add(LR, PP, ShifterOperand(LR));
     }
-    ldr(LR, Address(LR, offset12_lo));
+    ldr(LR, Address(LR, offset_lo));
   }
   blx(LR);  // Use blx instruction so that the return branch prediction works.
 }
 
 
 void Assembler::BranchLinkStore(const ExternalLabel* label, Address ad) {
   // TODO(regis): Revisit this code sequence.
   LoadImmediate(IP, label->address());  // Target address is never patched.
   str(PC, ad);
   blx(IP);  // Use blx instruction so that the return branch prediction works.
 }
 
 
 void Assembler::BranchLinkOffset(Register base, int offset) {
   ASSERT(base != PC);
   ASSERT(base != IP);
-  if (Address::CanHoldLoadOffset(kLoadWord, offset)) {
+  int32_t offset_mask;
+  if (Address::CanHoldLoadOffset(kLoadWord, offset, &offset_mask)) {
     ldr(IP, Address(base, offset));
   } else {
-    int offset_hi = offset & ~kOffset12Mask;
-    int offset_lo = offset & kOffset12Mask;
+    int offset_hi = offset & ~offset_mask;
+    int offset_lo = offset & offset_mask;

[zra, 2013/03/06 16:41:27]

For consistency, you might put ASSERT(CanHoldLoadOffset(..., offset_lo)) here
also like you have down in LoadFromOffset. Same in BranchLinkPatchable.

[regis, 2013/03/06 18:42:00]

I am all for consistency. So I'll remove the extra ones, instead of adding
redundant ones.

     ShifterOperand offset_hi_op;
     if (ShifterOperand::CanHold(offset_hi, &offset_hi_op)) {
       add(IP, base, offset_hi_op);
       ldr(IP, Address(IP, offset_lo));
     } else {
       LoadImmediate(IP, offset_hi);
       add(IP, IP, ShifterOperand(base));
       ldr(IP, Address(IP, offset_lo));
     }
   }
@@ skipping 41 matching lines @@
     vmovdrr(dd, IP, scratch, cond);
   }
 }
 
 
 void Assembler::LoadFromOffset(LoadOperandType type,
                                Register reg,
                                Register base,
                                int32_t offset,
                                Condition cond) {
-  if (!Address::CanHoldLoadOffset(type, offset)) {
+  int32_t offset_mask;
+  if (!Address::CanHoldLoadOffset(type, offset, &offset_mask)) {
     ASSERT(base != IP);
-    LoadImmediate(IP, offset, cond);
-    add(IP, IP, ShifterOperand(base), cond);
+    AddImmediate(IP, base, offset & ~offset_mask, cond);
     base = IP;
-    offset = 0;
+    offset = offset & offset_mask;
   }
-  ASSERT(Address::CanHoldLoadOffset(type, offset));
+  ASSERT(Address::CanHoldLoadOffset(type, offset, &offset_mask));
   switch (type) {
     case kLoadSignedByte:
       ldrsb(reg, Address(base, offset), cond);
       break;
     case kLoadUnsignedByte:
       ldrb(reg, Address(base, offset), cond);
       break;
     case kLoadSignedHalfword:
       ldrsh(reg, Address(base, offset), cond);
       break;
@@ skipping 10 matching lines @@
       UNREACHABLE();
   }
 }
 
 
 void Assembler::StoreToOffset(StoreOperandType type,
                               Register reg,
                               Register base,
                               int32_t offset,
                               Condition cond) {
-  if (!Address::CanHoldStoreOffset(type, offset)) {
+  int32_t offset_mask;
+  if (!Address::CanHoldStoreOffset(type, offset, &offset_mask)) {
     ASSERT(reg != IP);
     ASSERT(base != IP);
-    LoadImmediate(IP, offset, cond);
-    add(IP, IP, ShifterOperand(base), cond);
+    AddImmediate(IP, base, offset & ~offset_mask, cond);
     base = IP;
-    offset = 0;
+    offset = offset & offset_mask;
   }
-  ASSERT(Address::CanHoldStoreOffset(type, offset));
+  ASSERT(Address::CanHoldStoreOffset(type, offset, &offset_mask));
   switch (type) {
     case kStoreByte:
       strb(reg, Address(base, offset), cond);
       break;
     case kStoreHalfword:
       strh(reg, Address(base, offset), cond);
       break;
     case kStoreWord:
       str(reg, Address(base, offset), cond);
       break;
     case kStoreWordPair:
       strd(reg, Address(base, offset), cond);
       break;
     default:
       UNREACHABLE();
   }
 }
 
 
 void Assembler::LoadSFromOffset(SRegister reg,
                                 Register base,
                                 int32_t offset,
                                 Condition cond) {
-  if (!Address::CanHoldLoadOffset(kLoadSWord, offset)) {
+  int32_t offset_mask;
+  if (!Address::CanHoldLoadOffset(kLoadSWord, offset, &offset_mask)) {
     ASSERT(base != IP);
-    LoadImmediate(IP, offset, cond);
-    add(IP, IP, ShifterOperand(base), cond);
+    AddImmediate(IP, base, offset & ~offset_mask, cond);
     base = IP;
-    offset = 0;
+    offset = offset & offset_mask;
   }
-  ASSERT(Address::CanHoldLoadOffset(kLoadSWord, offset));
+  ASSERT(Address::CanHoldLoadOffset(kLoadSWord, offset, &offset_mask));
   vldrs(reg, Address(base, offset), cond);
 }
 
 
 void Assembler::StoreSToOffset(SRegister reg,
                                Register base,
                                int32_t offset,
                                Condition cond) {
-  if (!Address::CanHoldStoreOffset(kStoreSWord, offset)) {
+  int32_t offset_mask;
+  if (!Address::CanHoldStoreOffset(kStoreSWord, offset, &offset_mask)) {
     ASSERT(base != IP);
-    LoadImmediate(IP, offset, cond);
-    add(IP, IP, ShifterOperand(base), cond);
+    AddImmediate(IP, base, offset & ~offset_mask, cond);
     base = IP;
-    offset = 0;
+    offset = offset & offset_mask;
   }
-  ASSERT(Address::CanHoldStoreOffset(kStoreSWord, offset));
+  ASSERT(Address::CanHoldStoreOffset(kStoreSWord, offset, &offset_mask));
   vstrs(reg, Address(base, offset), cond);
 }
 
 
 void Assembler::LoadDFromOffset(DRegister reg,
                                 Register base,
                                 int32_t offset,
                                 Condition cond) {
-  if (!Address::CanHoldLoadOffset(kLoadDWord, offset)) {
+  int32_t offset_mask;
+  if (!Address::CanHoldLoadOffset(kLoadDWord, offset, &offset_mask)) {
     ASSERT(base != IP);
-    LoadImmediate(IP, offset, cond);
-    add(IP, IP, ShifterOperand(base), cond);
+    AddImmediate(IP, base, offset & ~offset_mask, cond);
     base = IP;
-    offset = 0;
+    offset = offset & offset_mask;
   }
-  ASSERT(Address::CanHoldLoadOffset(kLoadDWord, offset));
+  ASSERT(Address::CanHoldLoadOffset(kLoadDWord, offset, &offset_mask));
   vldrd(reg, Address(base, offset), cond);
 }
 
 
 void Assembler::StoreDToOffset(DRegister reg,
                                Register base,
                                int32_t offset,
                                Condition cond) {
-  if (!Address::CanHoldStoreOffset(kStoreDWord, offset)) {
+  int32_t offset_mask;
+  if (!Address::CanHoldStoreOffset(kStoreDWord, offset, &offset_mask)) {
     ASSERT(base != IP);
-    LoadImmediate(IP, offset, cond);
-    add(IP, IP, ShifterOperand(base), cond);
+    AddImmediate(IP, base, offset & ~offset_mask, cond);
     base = IP;
-    offset = 0;
+    offset = offset & offset_mask;
   }
-  ASSERT(Address::CanHoldStoreOffset(kStoreDWord, offset));
+  ASSERT(Address::CanHoldStoreOffset(kStoreDWord, offset, &offset_mask));
   vstrd(reg, Address(base, offset), cond);
 }
 
 
 void Assembler::AddImmediate(Register rd, int32_t value, Condition cond) {
   AddImmediate(rd, rd, value, cond);
 }
 
 
 void Assembler::AddImmediate(Register rd, Register rn, int32_t value,
@@ skipping 145 matching lines @@
     bic(SP, SP, ShifterOperand(OS::ActivationFrameAlignment() - 1));
   }
 }
 
 
 void Assembler::EnterCallRuntimeFrame(intptr_t frame_space) {
   // Preserve volatile CPU registers.
   EnterFrame(kDartVolatileCpuRegs | (1 << FP), 0);
 
   // Preserve all volatile FPU registers.
-  // TODO(regis): Use vstmd instruction once supported.
-  // vstmd(DB_W, SP, kDartFirstVolatileFpuReg, kDartLastVolatileFpuReg);
+  vstmd(DB_W, SP, kDartFirstVolatileFpuReg, kDartLastVolatileFpuReg);
 
   ReserveAlignedFrameSpace(frame_space);
 }
 
 
 void Assembler::LeaveCallRuntimeFrame() {
   // SP might have been modified to reserve space for arguments
   // and ensure proper alignment of the stack frame.
   // We need to restore it before restoring registers.
   const intptr_t kPushedRegistersSize =
-      kDartVolatileCpuRegCount * kWordSize;
-  // TODO(regis): + kDartVolatileFpuRegCount * 2 * kWordSize;
+      kDartVolatileCpuRegCount * kWordSize +
+      kDartVolatileFpuRegCount * 2 * kWordSize;
   AddImmediate(SP, FP, -kPushedRegistersSize);
 
   // Restore all volatile FPU registers.
-  // TODO(regis): Use vldmd instruction once supported.
-  // vldmd(IA_W, SP, kDartFirstVolatileFpuReg, kDartLastVolatileFpuReg);
+  vldmd(IA_W, SP, kDartFirstVolatileFpuReg, kDartLastVolatileFpuReg);
 
   // Restore volatile CPU registers.
   LeaveFrame(kDartVolatileCpuRegs | (1 << FP));
 }
 
 
 void Assembler::CallRuntime(const RuntimeEntry& entry) {
   entry.Call(this);
 }
 
@@ skipping 120 matching lines @@
   // Do not reuse an existing entry, since each reference may be patched
   // independently.
   object_pool_.Add(smi);
   return object_pool_.Length() - 1;
 }
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_ARM