More improvements to ARM array load code.

runtime/vm/intermediate_language_arm.cc

Index: runtime/vm/intermediate_language_arm.cc
===================================================================
--- runtime/vm/intermediate_language_arm.cc	(revision 36157)
+++ runtime/vm/intermediate_language_arm.cc	(working copy)
@@ -1135,7 +1135,11 @@
   // 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)   ||
@@ -1233,36 +1237,54 @@
   // 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: