Split array loads/stores for growable arrays into two IL instructions.

runtime/vm/intermediate_language_ia32.cc

 // Copyright (c) 2012, 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_IA32.
 #if defined(TARGET_ARCH_IA32)
 
 #include "vm/intermediate_language.h"
 
 #include "lib/error.h"
@@ skipping 847 matching lines @@
   __ movl(EDX, Immediate(arg_count));
   compiler->GenerateCall(token_pos(),
                          &StubCode::CallNativeCFunctionLabel(),
                          PcDescriptors::kOther,
                          locs());
   __ popl(result);
 }
 
 
 LocationSummary* LoadIndexedInstr::MakeLocationSummary() const {
-  ASSERT((receiver_type() == kGrowableObjectArrayCid) ||
-         (receiver_type() == kArrayCid) ||
-         (receiver_type() == kImmutableArrayCid));

[srdjan, 2012/09/12 07:06:40]

Why is this assert removed? Shouldn't it check that type is either kArrayCid or
kImmutableArrayCid? MAybe the assert belongs to EmitNativeCode?

[Florian Schneider, 2012/09/12 08:27:29]

I removed receiver_type() from LoadIndexed completely since it is not needed
anymore.

   const intptr_t kNumInputs = 2;
-  const intptr_t kNumTemps = receiver_type() == kGrowableObjectArrayCid ? 1 : 0;
-  LocationSummary* locs =
-      new LocationSummary(kNumInputs, kNumTemps, LocationSummary::kNoCall);
-  locs->set_in(0, Location::RequiresRegister());
-  locs->set_in(1, Location::RequiresRegister());
-  if (receiver_type() == kGrowableObjectArrayCid) {
-    locs->set_temp(0, Location::RequiresRegister());
-  }
-  locs->set_out(Location::RequiresRegister());
-  return locs;
+  return LocationSummary::Make(kNumInputs,
+                               Location::RequiresRegister(),
+                               LocationSummary::kNoCall);
 }
 
 
 void LoadIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
-  Register receiver = locs()->in(0).reg();
+  Register array = locs()->in(0).reg();
   Register index = locs()->in(1).reg();
   Register result = locs()->out().reg();
 
-  switch (receiver_type()) {
-    case kArrayCid:
-    case kImmutableArrayCid:
-      // Note that index is Smi, i.e, times 2.
-      ASSERT(kSmiTagShift == 1);
-      __ movl(result, FieldAddress(receiver, index, TIMES_2, sizeof(RawArray)));
-      break;
-
-    case kGrowableObjectArrayCid: {
-      Register temp = locs()->temp(0).reg();
-      __ movl(temp, FieldAddress(receiver, GrowableObjectArray::data_offset()));
-      // Note that index is Smi, i.e, times 2.
-      ASSERT(kSmiTagShift == 1);
-      __ movl(result, FieldAddress(temp, index, TIMES_2, sizeof(RawArray)));
-      break;
-    }
-
-    default:
-      UNREACHABLE();
-      break;
-  }
+  // Note that index is Smi, i.e, times 2.
+  ASSERT(kSmiTagShift == 1);
+  __ movl(result, FieldAddress(array, index, TIMES_2, sizeof(RawArray)));
 }
 
 
 LocationSummary* StoreIndexedInstr::MakeLocationSummary() const {
-  ASSERT((receiver_type() == kGrowableObjectArrayCid) ||
-         (receiver_type() == kArrayCid));

[srdjan, 2012/09/12 07:06:40]

Why is this assert removed? Shouldn't we check for kArrayCid only?

[Florian Schneider, 2012/09/12 08:27:29]

I removed receiver_type() from StoreIndexed as well.

   const intptr_t kNumInputs = 3;
-  const intptr_t kNumTemps = receiver_type() == kGrowableObjectArrayCid ? 1 : 0;
+  const intptr_t kNumTemps = 0;
   LocationSummary* locs =
       new LocationSummary(kNumInputs, kNumTemps, LocationSummary::kNoCall);
   locs->set_in(0, Location::RequiresRegister());
   locs->set_in(1, Location::RequiresRegister());
   locs->set_in(2, value()->NeedsStoreBuffer() ? Location::WritableRegister()
                                               : Location::RequiresRegister());
-  if (receiver_type() == kGrowableObjectArrayCid) {
-    locs->set_temp(0, Location::RequiresRegister());
-  }
   return locs;
 }
 
 
 void StoreIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
-  Register receiver = locs()->in(0).reg();
+  Register array = locs()->in(0).reg();
   Register index = locs()->in(1).reg();
   Register value = locs()->in(2).reg();
 
-  switch (receiver_type()) {
-    case kArrayCid:
-    case kImmutableArrayCid:
-      // Note that index is Smi, i.e, times 2.
-      ASSERT(kSmiTagShift == 1);
-      if (this->value()->NeedsStoreBuffer()) {
-        __ StoreIntoObject(receiver,
-            FieldAddress(receiver, index, TIMES_2, sizeof(RawArray)),
-            value);
-      } else {
-        __ StoreIntoObjectNoBarrier(receiver,
-            FieldAddress(receiver, index, TIMES_2, sizeof(RawArray)),
-            value);
-      }
-      break;
-
-    case kGrowableObjectArrayCid: {
-      Register temp = locs()->temp(0).reg();
-      __ movl(temp, FieldAddress(receiver, GrowableObjectArray::data_offset()));
-      // Note that index is Smi, i.e, times 2.
-      ASSERT(kSmiTagShift == 1);
-      if (this->value()->NeedsStoreBuffer()) {
-        __ StoreIntoObject(temp,
-            FieldAddress(temp, index, TIMES_2, sizeof(RawArray)),
-            value);
-      } else {
-        __ StoreIntoObjectNoBarrier(temp,
-            FieldAddress(temp, index, TIMES_2, sizeof(RawArray)),
-            value);
-      }
-      break;
-    }
-
-    default:
-      UNREACHABLE();
-      break;
+  // Note that index is Smi, i.e, times 2.
+  ASSERT(kSmiTagShift == 1);
+  if (this->value()->NeedsStoreBuffer()) {
+    __ StoreIntoObject(array,
+        FieldAddress(array, index, TIMES_2, sizeof(RawArray)),
+        value);
+  } else {
+    __ StoreIntoObjectNoBarrier(array,
+        FieldAddress(array, index, TIMES_2, sizeof(RawArray)),
+        value);
   }
 }
 
 
 LocationSummary* LoadInstanceFieldInstr::MakeLocationSummary() const {
   // TODO(fschneider): For this instruction the input register may be
   // reused for the result (but is not required to) because the input
   // is not used after the result is defined.  We should consider adding
   // this information to the input policy.
   return LocationSummary::Make(1,
@@ skipping 1302 matching lines @@
     __ j(ABOVE_EQUAL, deopt);
   }
 }
 
 
 }  // namespace dart
 
 #undef __
 
 #endif  // defined TARGET_ARCH_X64