More improvements to ARM array load code.

runtime/vm/intermediate_language_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"  // Needed here to get TARGET_ARCH_ARM.
 #if defined(TARGET_ARCH_ARM)
 
 #include "vm/intermediate_language.h"
 
 #include "vm/cpu.h"
@@ skipping 1117 matching lines @@
 
 LocationSummary* LoadIndexedInstr::MakeLocationSummary(bool opt) const {
   const intptr_t kNumInputs = 2;
   const intptr_t kNumTemps = 0;
   LocationSummary* locs =
       new LocationSummary(kNumInputs, kNumTemps, LocationSummary::kNoCall);
   locs->set_in(0, Location::RequiresRegister());
   // The smi index is either untagged (element size == 1), or it is left smi
   // tagged (for all element sizes > 1).
   // TODO(regis): Revisit and see if the index can be immediate.
-  locs->set_in(1, Location::WritableRegister());
+  if (index_scale() == 2 && IsExternal()) {
+    locs->set_in(1, Location::RequiresRegister());
+  } else {
+    locs->set_in(1, Location::WritableRegister());
+  }
   if ((representation() == kUnboxedDouble)    ||
       (representation() == kUnboxedFloat32x4) ||
       (representation() == kUnboxedInt32x4)   ||
       (representation() == kUnboxedFloat64x2)) {
     if (class_id() == kTypedDataFloat32ArrayCid) {
       // Need register <= Q7 for float operations.
       // TODO(fschneider): Add a register policy to specify a subset of
       // registers.
       locs->set_out(0, Location::FpuRegisterLocation(Q7));
     } else {
@@ skipping 77 matching lines @@
 
   Register array = locs()->in(0).reg();
   Location index = locs()->in(1);
 
   Address element_address(kNoRegister, 0);
   ASSERT(index.IsRegister());  // TODO(regis): Revisit.
   // Note that index is expected smi-tagged, (i.e, times 2) for all arrays
   // with index scale factor > 1. E.g., for Uint8Array and OneByteString the
   // index is expected to be untagged before accessing.
   ASSERT(kSmiTagShift == 1);
+  intptr_t offset = 0;
   switch (index_scale()) {
     case 1: {
-      __ SmiUntag(index.reg());
+      __ add(index.reg(), array, ShifterOperand(index.reg(), ASR, kSmiTagSize));
+      if (!IsExternal()) {

[zra, 2014/05/14 16:32:44]

Looks like you can hoist these out of the switch statement.

[Florian Schneider, 2014/05/15 09:43:27]

Done.

+        offset = FlowGraphCompiler::DataOffsetFor(class_id()) - kHeapObjectTag;
+      }
+      element_address = Address(index.reg(), offset);

[zra, 2014/05/14 16:32:44]

Same comment as in the other CL for the range of offset.

[Florian Schneider, 2014/05/15 09:43:27]

Done.

       break;
     }
     case 2: {
+      if (!IsExternal()) {
+        __ AddImmediate(index.reg(), index.reg(),
+            FlowGraphCompiler::DataOffsetFor(class_id()) - kHeapObjectTag);
+        element_address = Address(array, index.reg(), LSL, 0);
+      } else {
+        element_address = Address(array, index.reg(), LSL, 0);
+      }
       break;
     }
     case 4: {
-      __ mov(index.reg(), ShifterOperand(index.reg(), LSL, 1));
+      __ add(index.reg(), array, ShifterOperand(index.reg(), LSL, 1));
+      if (!IsExternal()) {
+        offset = FlowGraphCompiler::DataOffsetFor(class_id()) - kHeapObjectTag;
+      }
+      element_address = Address(index.reg(), offset);
       break;
     }
     case 8: {
-      __ mov(index.reg(), ShifterOperand(index.reg(), LSL, 2));
+      __ add(index.reg(), array, ShifterOperand(index.reg(), LSL, 2));
+      if (!IsExternal()) {
+        offset = FlowGraphCompiler::DataOffsetFor(class_id()) - kHeapObjectTag;
+      }
+      element_address = Address(index.reg(), offset);
       break;
     }
     case 16: {
-      __ mov(index.reg(), ShifterOperand(index.reg(), LSL, 3));
+      __ add(index.reg(), array, ShifterOperand(index.reg(), LSL, 3));
+      if (!IsExternal()) {
+        offset = FlowGraphCompiler::DataOffsetFor(class_id()) - kHeapObjectTag;
+      }
+      element_address = Address(index.reg(), offset);
       break;
     }
     default:
       UNREACHABLE();
   }
 
-  if (!IsExternal()) {
-    ASSERT(this->array()->definition()->representation() == kTagged);
-    __ AddImmediate(index.reg(),
-        FlowGraphCompiler::DataOffsetFor(class_id()) - kHeapObjectTag);
-  }
-  element_address = Address(array, index.reg(), LSL, 0);
   Register result = locs()->out(0).reg();
   switch (class_id()) {
     case kTypedDataInt8ArrayCid:
       ASSERT(index_scale() == 1);
       __ ldrsb(result, element_address);
       __ SmiTag(result);
       break;
     case kTypedDataUint8ArrayCid:
     case kTypedDataUint8ClampedArrayCid:
     case kExternalTypedDataUint8ArrayCid:
@@ skipping 4887 matching lines @@
   compiler->GenerateCall(token_pos(),
                          &label,
                          PcDescriptors::kOther,
                          locs());
   __ Drop(ArgumentCount());  // Discard arguments.
 }
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_ARM