Implement many KeyedStoreStubs using Crankshaft

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index c2f3951494e647936183cdb1d25659c8d5ee187f..d18c8449f10304238d67029c3d985dd347a86917 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -114,6 +114,8 @@ void HBasicBlock::AddInstruction(HInstruction* instr) {
   ASSERT(!instr->IsLinked());
   ASSERT(!IsFinished());
   if (first_ == NULL) {
+    ASSERT(last_environment() != NULL);
+    ASSERT(!last_environment()->ast_id().IsNone());
     HBlockEntry* entry = new(zone()) HBlockEntry();
     entry->InitializeAsFirst(this);
     first_ = last_ = entry;
@@ -231,6 +233,7 @@ void HBasicBlock::SetJoinId(BailoutId ast_id) {
             predecessor->last_environment()->closure()->shared()
               ->VerifyBailoutId(ast_id)));
     simulate->set_ast_id(ast_id);
+    predecessor->last_environment()->set_ast_id(ast_id);
   }
 }
 
@@ -619,6 +622,11 @@ HConstant* HGraph::GetConstant1() {
 }
 
 
+HConstant* HGraph::GetConstant2() {
+  return GetConstantInt32(&constant_2_, 1);

[Jakob Kummerow, 2013/03/11 16:36:07]

1 is the new 2?

[danno, 2013/03/13 15:36:26]

Done.

+}
+
+
 HConstant* HGraph::GetConstantMinus1() {
   return GetConstantInt32(&constant_minus1_, -1);
 }
@@ -700,7 +708,7 @@ HGraphBuilder::IfBuilder::IfBuilder(HGraphBuilder* builder, BailoutId id)
 }
 
 
-HInstruction* HGraphBuilder::IfBuilder::BeginTrue(
+HInstruction* HGraphBuilder::IfBuilder::BeginIf(
     HValue* left,
     HValue* right,
     Token::Value token,
@@ -718,7 +726,20 @@ HInstruction* HGraphBuilder::IfBuilder::BeginTrue(
 }
 
 
-void HGraphBuilder::IfBuilder::BeginFalse() {
+HInstruction* HGraphBuilder::IfBuilder::BeginIfObjectsEqual(
+    HValue* left,
+    HValue* right) {
+  HCompareObjectEqAndBranch* compare =
+      new(zone()) HCompareObjectEqAndBranch(left, right);
+  compare->SetSuccessorAt(0, first_true_block_);
+  compare->SetSuccessorAt(1, first_false_block_);
+  builder_->current_block()->Finish(compare);
+  builder_->set_current_block(first_true_block_);
+  return compare;
+}
+
+
+void HGraphBuilder::IfBuilder::BeginElse() {
   last_true_block_ = builder_->current_block();
   ASSERT(!last_true_block_->IsFinished());
   builder_->set_current_block(first_false_block_);
@@ -731,7 +752,7 @@ void HGraphBuilder::IfBuilder::End() {
   HBasicBlock* last_false_block = builder_->current_block();
   ASSERT(!last_false_block->IsFinished());
   HEnvironment* merge_env =
-      last_true_block_->last_environment()->Copy();
+      last_true_block_->last_environment()->CopyWithoutHistory();
   merge_block_ = builder_->CreateBasicBlock(merge_env);
   last_true_block_->Goto(merge_block_);
   last_false_block->Goto(merge_block_);
@@ -853,6 +874,7 @@ void HGraphBuilder::AddSimulate(BailoutId id,
                                 RemovableSimulate removable) {
   ASSERT(current_block() != NULL);
   current_block()->AddSimulate(id, removable);
+  environment()->set_ast_id(id);
 }
 
 
@@ -950,7 +972,8 @@ HInstruction* HGraphBuilder::BuildFastElementAccess(
     HValue* val,
     HValue* load_dependency,
     ElementsKind elements_kind,
-    bool is_store) {
+    bool is_store,
+    KeyedAccessStoreMode store_mode) {
   Zone* zone = this->zone();
   if (is_store) {
     ASSERT(val != NULL);
@@ -978,6 +1001,106 @@ HInstruction* HGraphBuilder::BuildFastElementAccess(
 }
 
 
+HValue* HGraphBuilder::BuildCheckForCapacityGrow(HValue* object,
+                                                 HValue* elements,
+                                                 ElementsKind kind,
+                                                 HValue* length,
+                                                 HValue* key,
+                                                 bool is_js_array) {
+  BailoutId ast_id = environment()->ast_id();
+  Zone* zone = this->zone();
+  IfBuilder length_checker_if(this, ast_id);

[Jakob Kummerow, 2013/03/11 16:36:07]

nit: Drop the _if. Just length_checker. It's cleaner.

[danno, 2013/03/13 15:36:26]

Done.

+
+  length_checker_if.BeginIf(length, key, Token::EQ);
+
+  HValue* current_capacity =
+      AddInstruction(new(zone) HFixedArrayBaseLength(elements));
+
+  IfBuilder capacity_checker_if(this, ast_id);

[Jakob Kummerow, 2013/03/11 16:36:07]

again

[danno, 2013/03/13 15:36:26]

Done.

+
+  capacity_checker_if.BeginIf(length, current_capacity, Token::EQ);
+
+  HValue* context = environment()->LookupContext();
+
+  HValue* new_capacity =
+      BuildNewElementsCapacity(context, current_capacity);
+
+  HValue* new_elements = BuildGrowElementsCapacity(object, elements,
+                                                   kind, length,
+                                                   new_capacity, ast_id);
+
+  environment()->Push(new_elements);
+  capacity_checker_if.BeginElse();
+
+  environment()->Push(elements);
+  capacity_checker_if.End();
+
+  if (is_js_array) {
+    HValue* new_length = AddInstruction(
+        HAdd::New(zone, context, length, graph_->GetConstant1()));
+    new_length->ChangeRepresentation(Representation::Integer32());
+    new_length->ClearFlag(HValue::kCanOverflow);
+    AddSimulate(ast_id, REMOVABLE_SIMULATE);
+
+    Isolate* isolate = graph()->isolate();

[Jakob Kummerow, 2013/03/11 16:36:07]

no need for this, HGraphBuilder has an isolate() accessor.

[danno, 2013/03/13 15:36:26]

Done.

+    Factory* factory = isolate->factory();
+    HInstruction* length_store = AddInstruction(new(zone) HStoreNamedField(
+        object,
+        factory->length_field_string(),
+        new_length, true,
+        JSArray::kLengthOffset));
+    length_store->SetGVNFlag(kChangesArrayLengths);
+    AddSimulate(ast_id, REMOVABLE_SIMULATE);
+  }
+
+  length_checker_if.BeginElse();
+
+  AddBoundsCheck(key, length, ALLOW_SMI_KEY);
+  environment()->Push(elements);
+
+  length_checker_if.End();
+
+  return environment()->Pop();
+}
+
+
+HValue* HGraphBuilder::BuildCopyElementsOnWrite(HValue* object,
+                                                HValue* elements,
+                                                ElementsKind kind,
+                                                HValue* length) {
+  BailoutId ast_id = environment()->ast_id();
+  Zone* zone = this->zone();
+  Heap* heap = info_->isolate()->heap();

[Jakob Kummerow, 2013/03/11 16:36:07]

s/info_->// (or s/info_/this/ if you like needless verbosity)

[danno, 2013/03/13 15:36:26]

Done.

+  HValue* elements_map =
+      AddInstruction(new(zone) HLoadNamedField(elements, true,

[Jakob Kummerow, 2013/03/11 16:36:07]

Are you intentionally not using an HCompareMap instruction here?

[danno, 2013/03/13 15:36:26]

Done.

+                                               JSObject::kMapOffset));
+  Handle<Map> map(Handle<Map>(heap->fixed_cow_array_map()));
+  HValue* cow_map =
+      AddInstruction(new(zone) HConstant(map, Representation::Tagged()));
+
+  IfBuilder cow_checker(this, ast_id);
+
+  cow_checker.BeginIfObjectsEqual(elements_map, cow_map);
+
+  HValue* capacity =
+      AddInstruction(new(zone) HFixedArrayBaseLength(elements));
+
+  HValue* new_elements = BuildGrowElementsCapacity(object, elements,
+                                                   kind, length,
+                                                   capacity, ast_id);
+
+  environment()->Push(new_elements);
+
+  cow_checker.BeginElse();
+
+  environment()->Push(elements);
+
+  cow_checker.End();
+
+  return environment()->Pop();
+}
+
+
 HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
     HValue* object,
     HValue* key,
@@ -986,7 +1109,9 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
     bool is_js_array,
     ElementsKind elements_kind,
     bool is_store,
+    KeyedAccessStoreMode store_mode,
     Representation checked_index_representation) {
+  ASSERT(!IsExternalArrayElementsKind(elements_kind) || !is_js_array);
   Zone* zone = this->zone();
   // No GVNFlag is necessary for ElementsKind if there is an explicit dependency
   // on a HElementsTransition instruction. The flag can also be removed if the
@@ -1002,46 +1127,104 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
   }
   bool fast_smi_only_elements = IsFastSmiElementsKind(elements_kind);
   bool fast_elements = IsFastObjectElementsKind(elements_kind);
-  HInstruction* elements =
+  HValue* elements =
       AddInstruction(new(zone) HLoadElements(object, mapcheck));
-  if (is_store && (fast_elements || fast_smi_only_elements)) {
+  if (is_store && (fast_elements || fast_smi_only_elements) &&
+      store_mode != STORE_NO_TRANSITION_HANDLE_COW) {
     HCheckMaps* check_cow_map = new(zone) HCheckMaps(
         elements, isolate()->factory()->fixed_array_map(), zone);
     check_cow_map->ClearGVNFlag(kDependsOnElementsKind);
     AddInstruction(check_cow_map);
   }
   HInstruction* length = NULL;
-  HInstruction* checked_key = NULL;
-  if (IsExternalArrayElementsKind(elements_kind)) {
+  if (is_js_array) {
+    length = AddInstruction(new(zone) HJSArrayLength(object, mapcheck,
+                                                     HType::Smi()));
+  } else {
     length = AddInstruction(new(zone) HFixedArrayBaseLength(elements));
-    checked_key = AddBoundsCheck(
-        key, length, ALLOW_SMI_KEY, checked_index_representation);
-    HLoadExternalArrayPointer* external_elements =
-        new(zone) HLoadExternalArrayPointer(elements);
-    AddInstruction(external_elements);
-    return BuildExternalArrayElementAccess(
-        external_elements, checked_key, val, mapcheck,
-        elements_kind, is_store);
+  }
+  HValue* checked_key = NULL;
+  if (IsExternalArrayElementsKind(elements_kind)) {
+    if (store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS) {
+      HLoadExternalArrayPointer* external_elements =
+          new(zone) HLoadExternalArrayPointer(elements);
+      AddInstruction(external_elements);
+      HCheckSmiOrInt32* checked_index =
+          new(graph()->zone()) HCheckSmiOrInt32(key);
+      AddInstruction(checked_index);
+      BailoutId previous_id = environment()->ast_id();
+      ASSERT(!previous_id.IsNone());
+      IfBuilder oob_ignore_check(this, previous_id);

[Jakob Kummerow, 2013/03/11 16:36:07]

How about we call this IfBuilder length_checker (analogously to
BuildCheckForCapacityGrow above)...

[danno, 2013/03/13 15:36:26]

Done.

+      oob_ignore_check.BeginIf(checked_index, length, Token::LT);
+      IfBuilder oob_ignore_check2(this, previous_id);

[Jakob Kummerow, 2013/03/11 16:36:07]

...and this one negative_checker?

[danno, 2013/03/13 15:36:26]

Done.

+      HInstruction* bounds_check =
+          oob_ignore_check2.BeginIf(checked_index,
+                                    graph()->GetConstant0(),
+                                    Token::LT);
+      // Negative array indices can't be ignored and must be handled by the
+      // runtime.
+      AddSoftDeoptimize();

[Jakob Kummerow, 2013/03/11 16:36:07]

You don't want a soft deopt here. HSoftDeoptimize is intended for blocks where
we don't have type feedback yet, not for things we generally can't handle in
optimized code; if FLAG_always_opt, AddSoftDeoptimize() does nothing.

You should use an HDeoptimize here instead, which you can insert via
current_block()->FinishExitWithDeoptimization. Too bad that doesn't fit into the
IfBuilder infrastructure.

Alternative: (ab)use an HBoundsCheck. It does an unsigned comparison, so if I'm
not mistaken you can get the desired behavior (deopt if checked_index < 0) with
a bounds check against 0x10000000 (aka kMinInt).

[danno, 2013/03/13 15:36:26]

Done.

+      oob_ignore_check2.BeginElse();
+      HInstruction* result = BuildExternalArrayElementAccess(
+          external_elements, key, val, bounds_check,
+          elements_kind, is_store);
+      AddInstruction(result);
+      oob_ignore_check2.End();
+      oob_ignore_check.BeginElse();
+      oob_ignore_check.End();
+      return result;
+    } else {
+      ASSERT(store_mode == STANDARD_STORE);
+      checked_key = AddBoundsCheck(
+          key, length, ALLOW_SMI_KEY, checked_index_representation);
+      HLoadExternalArrayPointer* external_elements =
+          new(zone) HLoadExternalArrayPointer(elements);
+      AddInstruction(external_elements);
+      return AddInstruction(BuildExternalArrayElementAccess(
+          external_elements, checked_key, val, mapcheck,
+          elements_kind, is_store));
+    }
   }
   ASSERT(fast_smi_only_elements ||
          fast_elements ||
          IsFastDoubleElementsKind(elements_kind));
-  if (is_js_array) {
-    length = AddInstruction(new(zone) HJSArrayLength(object, mapcheck,
-                                                     HType::Smi()));
+
+  if (is_store && IsFastSmiElementsKind(elements_kind) &&
+      val->type().IsTagged() && !val->type().IsSmi()) {

[Jakob Kummerow, 2013/03/11 16:36:07]

Fun fact: there is no HType (except "uninitialized") for which type().IsTagged()
would return "false".

[danno, 2013/03/13 15:36:26]

Done.

+    AddInstruction(new(zone) HCheckSmi(val));
+  }
+
+  if (IsGrowStoreMode(store_mode)) {
+    elements = BuildCheckForCapacityGrow(object, elements, elements_kind,
+                                         length, key, is_js_array);
+    checked_key = key;
   } else {
-    length = AddInstruction(new(zone) HFixedArrayBaseLength(elements));
+    checked_key = AddBoundsCheck(
+        key, length, ALLOW_SMI_KEY, checked_index_representation);
+
+    if (is_store && (fast_elements || fast_smi_only_elements)) {
+      if (store_mode == STORE_NO_TRANSITION_HANDLE_COW) {
+        elements =
+            BuildCopyElementsOnWrite(object, elements, elements_kind,
+                                     length);

[Jakob Kummerow, 2013/03/11 16:36:07]

nit: fits on the previous line

[danno, 2013/03/13 15:36:26]

Done.

+      } else {
+        HCheckMaps* check_cow_map = new(zone) HCheckMaps(
+            elements, Isolate::Current()->factory()->fixed_array_map(), zone);

[Jakob Kummerow, 2013/03/11 16:36:07]

s/Isolate::Current()/isolate()/

[danno, 2013/03/13 15:36:26]

Done.

+        check_cow_map->ClearGVNFlag(kDependsOnElementsKind);
+        AddInstruction(check_cow_map);
+      }
+    }
   }
-  checked_key = AddBoundsCheck(
-      key, length, ALLOW_SMI_KEY, checked_index_representation);
-  return BuildFastElementAccess(elements, checked_key, val, mapcheck,
-                                elements_kind, is_store);
+  return AddInstruction(
+      BuildFastElementAccess(elements, checked_key, val, mapcheck,
+                             elements_kind, is_store, store_mode));
 }
 
 
-HValue* HGraphBuilder::BuildAllocateElements(HContext* context,
+HValue* HGraphBuilder::BuildAllocateElements(HValue* context,
                                              ElementsKind kind,
-                                             HValue* capacity) {
+                                             HValue* capacity,
+                                             BailoutId ast_id) {
   Zone* zone = this->zone();
 
   int elements_size = IsFastDoubleElementsKind(kind)
@@ -1076,14 +1259,14 @@ HValue* HGraphBuilder::BuildAllocateElements(HContext* context,
   Handle<Map> map = IsFastDoubleElementsKind(kind)
       ? factory->fixed_double_array_map()
       : factory->fixed_array_map();
-  BuildStoreMap(elements, map, BailoutId::StubEntry());
+  BuildStoreMap(elements, map, ast_id);
 
   Handle<String> fixed_array_length_field_name = factory->length_field_string();
   HInstruction* store_length =
       new(zone) HStoreNamedField(elements, fixed_array_length_field_name,
                                  capacity, true, FixedArray::kLengthOffset);
   AddInstruction(store_length);
-  AddSimulate(BailoutId::StubEntry(), FIXED_SIMULATE);
+  AddSimulate(ast_id, REMOVABLE_SIMULATE);
 
   return elements;
 }
@@ -1100,7 +1283,7 @@ HInstruction* HGraphBuilder::BuildStoreMap(HValue* object,
                                  true, JSObject::kMapOffset);
   store_map->SetGVNFlag(kChangesMaps);
   AddInstruction(store_map);
-  AddSimulate(id, FIXED_SIMULATE);
+  AddSimulate(id, REMOVABLE_SIMULATE);
   return store_map;
 }
 
@@ -1115,17 +1298,135 @@ HInstruction* HGraphBuilder::BuildStoreMap(HValue* object,
 }
 
 
-void HGraphBuilder::BuildCopyElements(HContext* context,
+HValue* HGraphBuilder::BuildNewElementsCapacity(HValue* context,
+                                                HValue* old_capacity) {
+  Zone* zone = this->zone();
+  HValue* half_old_capacity =
+      AddInstruction(HDiv::New(zone,

[Jakob Kummerow, 2013/03/11 16:36:07]

prefer HSar (then you won't need GetConstant2() at all).

[danno, 2013/03/13 15:36:26]

Done.

+                               context,
+                               old_capacity,
+                               graph_->GetConstant2()));
+  half_old_capacity->ChangeRepresentation(Representation::Integer32());
+  half_old_capacity->ClearFlag(HValue::kCanOverflow);
+
+  HValue* new_capacity = AddInstruction(
+      HAdd::New(zone, context, half_old_capacity, old_capacity));
+  new_capacity->ChangeRepresentation(Representation::Integer32());
+  new_capacity->ClearFlag(HValue::kCanOverflow);
+
+  HValue* min_growth =
+      AddInstruction(new(zone) HConstant(4, Representation::Integer32()));

[Jakob Kummerow, 2013/03/11 16:36:07]

The C++ version (NewElementsCapacity in objects.h) adds 16 here.

[danno, 2013/03/13 15:36:26]

Done.

+
+  new_capacity = AddInstruction(
+      HAdd::New(zone, context, new_capacity, min_growth));
+  new_capacity->ChangeRepresentation(Representation::Integer32());
+  new_capacity->ClearFlag(HValue::kCanOverflow);
+
+  return new_capacity;
+}
+
+
+void HGraphBuilder::BuildNewSpaceArrayCheck(HValue* length, ElementsKind kind) {
+  Zone* zone = this->zone();
+  Heap* heap = info_->isolate()->heap();

[Jakob Kummerow, 2013/03/11 16:36:07]

s/info_->//

[danno, 2013/03/13 15:36:26]

Done.

+  int element_size = IsFastDoubleElementsKind(kind)
+      ? kDoubleSize

[Jakob Kummerow, 2013/03/11 16:36:07]

nit: fits on previous line. For your copy/paste convenience:

  int element_size = IsFastDoubleElementsKind(kind) ? kDoubleSize
                                                    : kPointerSize;

[danno, 2013/03/13 15:36:26]

Done.

+      : kPointerSize;
+  int max_size = heap->MaxNewSpaceAllocationSize() / element_size;
+  max_size -= sizeof(JSArray) / element_size;

[Jakob Kummerow, 2013/03/11 16:36:07]

I think you mean s/sizeof(JSArray)/JSArray::kSize/.

[danno, 2013/03/13 15:36:26]

Done.

+  HConstant* max_size_constant =
+      new(zone) HConstant(max_size, Representation::Integer32());
+  AddInstruction(max_size_constant);
+  // Since we're forcing Integer32 representation for this HBoundsCheck,

[Jakob Kummerow, 2013/03/11 16:36:07]

I'd prefer to make this explicit by passing (_, _, DONT_ALLOW_SMI_KEY,
Representation::Integer32()) to the ctor.

[danno, 2013/03/13 15:36:26]

Done.

+  // there's no need to Smi-check the index.
+  AddInstruction(new(zone) HBoundsCheck(length, max_size_constant));
+}
+
+
+HValue* HGraphBuilder::BuildGrowElementsCapacity(HValue* object,
+                                                 HValue* elements,
+                                                 ElementsKind kind,
+                                                 HValue* length,
+                                                 HValue* new_capacity,
+                                                 BailoutId ast_id) {
+  Zone* zone = this->zone();
+  HValue* context = environment()->LookupContext();
+
+  BuildNewSpaceArrayCheck(new_capacity, kind);
+
+  HValue* new_elements =
+      BuildAllocateElements(context, kind, new_capacity, ast_id);
+
+  BuildCopyElements(context, elements, kind,
+                    new_elements, kind,
+                    length, new_capacity, ast_id);
+
+  Isolate* isolate = graph()->isolate();

[Jakob Kummerow, 2013/03/11 16:36:07]

no need for this, just use [this->]isolate() directly.

[danno, 2013/03/13 15:36:26]

Done.

+  Factory* factory = isolate->factory();
+  HInstruction* elements_store = AddInstruction(new(zone) HStoreNamedField(
+      object,
+      factory->elements_field_string(),
+      new_elements, true,
+      JSArray::kElementsOffset));
+  elements_store->SetGVNFlag(kChangesElementsPointer);
+
+  return new_elements;
+}
+
+
+void HGraphBuilder::BuildFillElementsWithHole(HValue* context,
+                                              HValue* elements,
+                                              ElementsKind elements_kind,
+                                              HValue* from,
+                                              HValue* to,
+                                              BailoutId ast_id) {
+  // Fast elements kinds need to be initialized in case statements below cause
+  // a garbage collection.
+  Factory* factory = info_->isolate()->factory();

[Jakob Kummerow, 2013/03/11 16:36:07]

s/info_->//

[danno, 2013/03/13 15:36:26]

Done.

+
+  double nan_double = FixedDoubleArray::hole_nan_as_double();
+  Zone* zone = this->zone();
+  HValue* hole = IsFastSmiOrObjectElementsKind(elements_kind)
+      ? AddInstruction(new(zone) HConstant(factory->the_hole_value(),
+                                           Representation::Tagged()))
+      : AddInstruction(new(zone) HConstant(nan_double,
+                                           Representation::Double()));
+
+  LoopBuilder builder(this, context, LoopBuilder::kPostIncrement, ast_id);
+
+  HValue* key = builder.BeginBody(from, to, Token::LT);
+
+  AddInstruction(new(zone) HStoreKeyed(elements, key, hole,
+                                       elements_kind));

[Jakob Kummerow, 2013/03/11 16:36:07]

nit: fits on previous line

[danno, 2013/03/13 15:36:26]

Done.

+  AddSimulate(ast_id, REMOVABLE_SIMULATE);
+
+  builder.EndBody();
+}
+
+
+void HGraphBuilder::BuildCopyElements(HValue* context,
                                       HValue* from_elements,
                                       ElementsKind from_elements_kind,
                                       HValue* to_elements,
                                       ElementsKind to_elements_kind,
-                                      HValue* length) {
-  LoopBuilder builder(this, context, LoopBuilder::kPostIncrement,
-                      BailoutId::StubEntry());
+                                      HValue* length,
+                                      HValue* capacity,
+                                      BailoutId ast_id) {
+  bool pre_fill_with_holes =
+      IsFastDoubleElementsKind(from_elements_kind) &&
+      IsFastObjectElementsKind(to_elements_kind);
 
-  HValue* key = builder.BeginBody(graph()->GetConstant0(),
-                                  length, Token::LT);
+  if (pre_fill_with_holes) {
+    // If the copy might trigger a GC, make sure that the FixedArray is
+    // pre-initialized with holes to make sure that it's always in a consistent
+    // state.
+    BuildFillElementsWithHole(context, to_elements, to_elements_kind,
+                              graph()->GetConstant0(), capacity, ast_id);
+  }
+
+  LoopBuilder builder(this, context, LoopBuilder::kPostIncrement, ast_id);
+
+  HValue* key = builder.BeginBody(graph()->GetConstant0(), length, Token::LT);
 
   HValue* element =
       AddInstruction(new(zone()) HLoadKeyed(from_elements, key, NULL,
@@ -1134,9 +1435,24 @@ void HGraphBuilder::BuildCopyElements(HContext* context,
 
   AddInstruction(new(zone()) HStoreKeyed(to_elements, key, element,
                                          to_elements_kind));
-  AddSimulate(BailoutId::StubEntry(), REMOVABLE_SIMULATE);
+  AddSimulate(ast_id, REMOVABLE_SIMULATE);
 
   builder.EndBody();
+
+  if (!pre_fill_with_holes && length != capacity) {
+    // Fill unused capacity with the hole.
+    BuildFillElementsWithHole(context, to_elements, to_elements_kind,
+                              key, capacity, ast_id);
+  }
+}
+
+
+void HGraphBuilder::AddSoftDeoptimize() {
+  if (FLAG_always_opt) return;
+  if (current_block()->IsDeoptimizing()) return;
+  AddInstruction(new(zone()) HSoftDeoptimize());
+  current_block()->MarkAsDeoptimizing();
+  graph()->set_has_soft_deoptimize(true);
 }
 
 
@@ -4499,15 +4815,6 @@ void HOptimizedGraphBuilder::PushAndAdd(HInstruction* instr) {
 }
 
 
-void HOptimizedGraphBuilder::AddSoftDeoptimize() {
-  if (FLAG_always_opt) return;
-  if (current_block()->IsDeoptimizing()) return;
-  AddInstruction(new(zone()) HSoftDeoptimize());
-  current_block()->MarkAsDeoptimizing();
-  graph()->set_has_soft_deoptimize(true);
-}
-
-
 template <class Instruction>
 HInstruction* HOptimizedGraphBuilder::PreProcessCall(Instruction* call) {
   int count = call->argument_count();
@@ -6781,7 +7088,8 @@ HInstruction* HOptimizedGraphBuilder::BuildMonomorphicElementAccess(
     HValue* val,
     HValue* dependency,
     Handle<Map> map,
-    bool is_store) {
+    bool is_store,
+    KeyedAccessStoreMode store_mode) {
   HCheckMaps* mapcheck = new(zone()) HCheckMaps(object, map,
                                                 zone(), dependency);
   AddInstruction(mapcheck);
@@ -6791,7 +7099,7 @@ HInstruction* HOptimizedGraphBuilder::BuildMonomorphicElementAccess(
   return BuildUncheckedMonomorphicElementAccess(
       object, key, val,
       mapcheck, map->instance_type() == JS_ARRAY_TYPE,
-      map->elements_kind(), is_store);
+      map->elements_kind(), is_store, store_mode);
 }
 
 
@@ -6846,7 +7154,7 @@ HInstruction* HOptimizedGraphBuilder::TryBuildConsolidatedElementLoad(
       object, key, val, check_maps,
       most_general_consolidated_map->instance_type() == JS_ARRAY_TYPE,
       most_general_consolidated_map->elements_kind(),
-      false);
+      false, STANDARD_STORE);
   return instr;
 }
 
@@ -6859,6 +7167,7 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess(
     BailoutId ast_id,
     int position,
     bool is_store,
+    KeyedAccessStoreMode store_mode,
     bool* has_side_effects) {
   *has_side_effects = false;
   AddInstruction(new(zone()) HCheckNonSmi(object));
@@ -6869,7 +7178,6 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess(
     HInstruction* consolidated_load =
         TryBuildConsolidatedElementLoad(object, key, val, maps);
     if (consolidated_load != NULL) {
-      AddInstruction(consolidated_load);
       *has_side_effects |= consolidated_load->HasObservableSideEffects();
       if (position != RelocInfo::kNoPosition) {
         consolidated_load->set_position(position);
@@ -6936,8 +7244,9 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess(
       instr = AddInstruction(is_store ? BuildStoreKeyedGeneric(object, key, val)
                                       : BuildLoadKeyedGeneric(object, key));
     } else {
-      instr = AddInstruction(BuildMonomorphicElementAccess(
-          object, key, val, transition, untransitionable_map, is_store));
+      instr = BuildMonomorphicElementAccess(
+          object, key, val, transition, untransitionable_map, is_store,
+          store_mode);
     }
     *has_side_effects |= instr->HasObservableSideEffects();
     if (position != RelocInfo::kNoPosition) instr->set_position(position);
@@ -7018,7 +7327,7 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess(
         checked_key = AddBoundsCheck(key, length, ALLOW_SMI_KEY);
         access = AddInstruction(BuildFastElementAccess(
             elements, checked_key, val, elements_kind_branch,
-            elements_kind, is_store));
+            elements_kind, is_store, STANDARD_STORE));
         if (!is_store) {
           Push(access);
         }
@@ -7034,7 +7343,7 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess(
         checked_key = AddBoundsCheck(key, length, ALLOW_SMI_KEY);
         access = AddInstruction(BuildFastElementAccess(
             elements, checked_key, val, elements_kind_branch,
-            elements_kind, is_store));
+            elements_kind, is_store, STANDARD_STORE));
       } else if (elements_kind == DICTIONARY_ELEMENTS) {
         if (is_store) {
           access = AddInstruction(BuildStoreKeyedGeneric(object, key, val));
@@ -7080,23 +7389,27 @@ HValue* HOptimizedGraphBuilder::HandleKeyedElementAccess(
     if (map->has_slow_elements_kind()) {
       instr = is_store ? BuildStoreKeyedGeneric(obj, key, val)
                        : BuildLoadKeyedGeneric(obj, key);
+      AddInstruction(instr);
     } else {
       AddInstruction(new(zone()) HCheckNonSmi(obj));
-      instr = BuildMonomorphicElementAccess(obj, key, val, NULL, map, is_store);
+      instr = BuildMonomorphicElementAccess(
+          obj, key, val, NULL, map,
+          is_store, expr->GetStoreMode());

[Jakob Kummerow, 2013/03/11 16:36:07]

nit: fits on previous line

[danno, 2013/03/13 15:36:26]

Done.

     }
   } else if (expr->GetReceiverTypes() != NULL &&
              !expr->GetReceiverTypes()->is_empty()) {
     return HandlePolymorphicElementAccess(
-        obj, key, val, expr, ast_id, position, is_store, has_side_effects);
+        obj, key, val, expr, ast_id, position, is_store,
+        expr->GetStoreMode(), has_side_effects);
   } else {
     if (is_store) {
       instr = BuildStoreKeyedGeneric(obj, key, val);
     } else {
       instr = BuildLoadKeyedGeneric(obj, key);
     }
+    AddInstruction(instr);
   }
   if (position != RelocInfo::kNoPosition) instr->set_position(position);
-  AddInstruction(instr);
   *has_side_effects = instr->HasObservableSideEffects();
   return instr;
 }