Implement many KeyedStoreStubs using Crankshaft

src/hydrogen.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 96 matching lines @@
   phis_.RemoveElement(phi);
   phi->SetBlock(NULL);
 }
 
 
 void HBasicBlock::AddInstruction(HInstruction* instr) {
   ASSERT(!IsStartBlock() || !IsFinished());
   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;
   }
   instr->InsertAfter(last_);
 }
 
 
 HDeoptimize* HBasicBlock::CreateDeoptimize(
     HDeoptimize::UseEnvironment has_uses) {
@@ skipping 97 matching lines @@
   for (int i = 0; i < length; i++) {
     HBasicBlock* predecessor = predecessors_[i];
     ASSERT(predecessor->end()->IsGoto());
     HSimulate* simulate = HSimulate::cast(predecessor->end()->previous());
     // We only need to verify the ID once.
     ASSERT(i != 0 ||
            (predecessor->last_environment()->closure().is_null() ||
             predecessor->last_environment()->closure()->shared()
               ->VerifyBailoutId(ast_id)));
     simulate->set_ast_id(ast_id);
+    predecessor->last_environment()->set_ast_id(ast_id);
   }
 }
 
 
 bool HBasicBlock::Dominates(HBasicBlock* other) const {
   HBasicBlock* current = other->dominator();
   while (current != NULL) {
     if (current == this) return true;
     current = current->dominator();
   }
@@ skipping 368 matching lines @@
 HConstant* HGraph::GetConstant0() {
   return GetConstantInt32(&constant_0_, 0);
 }
 
 
 HConstant* HGraph::GetConstant1() {
   return GetConstantInt32(&constant_1_, 1);
 }
 
 
+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);
 }
 
 
 HConstant* HGraph::GetConstantTrue() {
   return GetConstant(&constant_true_, isolate()->factory()->true_value());
 }
 
 
@@ skipping 61 matching lines @@
     : builder_(builder),
       finished_(false),
       id_(id) {
   HEnvironment* env = builder->environment();
   first_true_block_ = builder->CreateBasicBlock(env->Copy());
   last_true_block_ = NULL;
   first_false_block_ = builder->CreateBasicBlock(env->Copy());
 }
 
 
-HInstruction* HGraphBuilder::IfBuilder::BeginTrue(
+HInstruction* HGraphBuilder::IfBuilder::BeginIf(
     HValue* left,
     HValue* right,
     Token::Value token,
     Representation input_representation) {
   HCompareIDAndBranch* compare =
       new(zone()) HCompareIDAndBranch(left, right, token);
   compare->set_observed_input_representation(input_representation,
                                              input_representation);
   compare->ChangeRepresentation(input_representation);
   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::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_);
 }
 
 
 void HGraphBuilder::IfBuilder::End() {
   ASSERT(!finished_);
   ASSERT(!last_true_block_->IsFinished());
   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_);
   merge_block_->SetJoinId(id_);
   builder_->set_current_block(merge_block_);
   finished_ = true;
 }
 
 
 HGraphBuilder::LoopBuilder::LoopBuilder(HGraphBuilder* builder,
@@ skipping 101 matching lines @@
   ASSERT(current_block() != NULL);
   current_block()->AddInstruction(instr);
   return instr;
 }
 
 
 void HGraphBuilder::AddSimulate(BailoutId id,
                                 RemovableSimulate removable) {
   ASSERT(current_block() != NULL);
   current_block()->AddSimulate(id, removable);
+  environment()->set_ast_id(id);
 }
 
 
 HBoundsCheck* HGraphBuilder::AddBoundsCheck(HValue* index,
                                             HValue* length,
                                             BoundsCheckKeyMode key_mode,
                                             Representation r) {
   if (!index->type().IsSmi()) {
     index = new(graph()->zone()) HCheckSmiOrInt32(index);
     AddInstruction(HCheckSmiOrInt32::cast(index));
@@ skipping 77 matching lines @@
   }
 }
 
 
 HInstruction* HGraphBuilder::BuildFastElementAccess(
     HValue* elements,
     HValue* checked_key,
     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);
     switch (elements_kind) {
       case FAST_SMI_ELEMENTS:
       case FAST_HOLEY_SMI_ELEMENTS:
         // Smi-only arrays need a smi check.
         AddInstruction(new(zone) HCheckSmi(val));
         // Fall through.
       case FAST_ELEMENTS:
       case FAST_HOLEY_ELEMENTS:
       case FAST_DOUBLE_ELEMENTS:
       case FAST_HOLEY_DOUBLE_ELEMENTS:
         return new(zone) HStoreKeyed(elements, checked_key, val, elements_kind);
       default:
         UNREACHABLE();
         return NULL;
     }
   }
   // It's an element load (!is_store).
   return new(zone) HLoadKeyed(elements,
                               checked_key,
                               load_dependency,
                               elements_kind);
 }
 
 
+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,
     HValue* val,
     HCheckMaps* mapcheck,
     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
   // map to check has FAST_HOLEY_ELEMENTS, since there can be no further
   // ElementsKind transitions. Finally, the dependency can be removed for stores
   // for FAST_ELEMENTS, since a transition to HOLEY elements won't change the
   // generated store code.
   if ((elements_kind == FAST_HOLEY_ELEMENTS) ||
       (elements_kind == FAST_ELEMENTS && is_store)) {
     if (mapcheck != NULL) {
       mapcheck->ClearGVNFlag(kDependsOnElementsKind);
     }
   }
   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)) {
-    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);
-  }
-  ASSERT(fast_smi_only_elements ||
-         fast_elements ||
-         IsFastDoubleElementsKind(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);
-  return BuildFastElementAccess(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_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 {
+    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);
+      }
+    }
+  }
+  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)
       ? kDoubleSize : kPointerSize;
   HConstant* elements_size_value =
       new(zone) HConstant(elements_size, Representation::Integer32());
   AddInstruction(elements_size_value);
   HValue* mul = AddInstruction(
                     HMul::New(zone, context, capacity, elements_size_value));
   mul->ChangeRepresentation(Representation::Integer32());
@@ skipping 14 matching lines @@
   }
 
   HValue* elements =
       AddInstruction(new(zone) HAllocate(context, total_size,
                                          HType::JSArray(), flags));
 
   Factory* factory = isolate()->factory();
   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;
 }
 
 
 HInstruction* HGraphBuilder::BuildStoreMap(HValue* object,
                                            HValue* map,
                                            BailoutId id) {
   Zone* zone = this->zone();
   Factory* factory = isolate()->factory();
   Handle<String> map_field_name = factory->map_field_string();
   HInstruction* store_map =
       new(zone) HStoreNamedField(object, map_field_name, map,
                                  true, JSObject::kMapOffset);
   store_map->SetGVNFlag(kChangesMaps);
   AddInstruction(store_map);
-  AddSimulate(id, FIXED_SIMULATE);
+  AddSimulate(id, REMOVABLE_SIMULATE);
   return store_map;
 }
 
 
 HInstruction* HGraphBuilder::BuildStoreMap(HValue* object,
                                            Handle<Map> map,
                                            BailoutId id) {
   Zone* zone = this->zone();
   HValue* map_constant =
       AddInstruction(new(zone) HConstant(map, Representation::Tagged()));
   return BuildStoreMap(object, map_constant, id);
 }
 
 
-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,
                                             from_elements_kind,
                                             ALLOW_RETURN_HOLE));
 
   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);
+}
+
+
 HOptimizedGraphBuilder::HOptimizedGraphBuilder(CompilationInfo* info,
                                                TypeFeedbackOracle* oracle)
     : HGraphBuilder(info),
       function_state_(NULL),
       initial_function_state_(this, info, oracle, NORMAL_RETURN),
       ast_context_(NULL),
       break_scope_(NULL),
       inlined_count_(0),
       globals_(10, info->zone()),
       inline_bailout_(false) {
@@ skipping 3339 matching lines @@
   current_block()->AddPhi(instr);
 }
 
 
 void HOptimizedGraphBuilder::PushAndAdd(HInstruction* instr) {
   Push(instr);
   AddInstruction(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();
   ZoneList<HValue*> arguments(count, zone());
   for (int i = 0; i < count; ++i) {
     arguments.Add(Pop(), zone());
   }
 
   while (!arguments.is_empty()) {
     AddInstruction(new(zone()) HPushArgument(arguments.RemoveLast()));
@@ skipping 2253 matching lines @@
   return new(zone()) HLoadKeyedGeneric(context, object, key);
 }
 
 
 HInstruction* HOptimizedGraphBuilder::BuildMonomorphicElementAccess(
     HValue* object,
     HValue* key,
     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);
   if (dependency) {
     mapcheck->ClearGVNFlag(kDependsOnElementsKind);
   }
   return BuildUncheckedMonomorphicElementAccess(
       object, key, val,
       mapcheck, map->instance_type() == JS_ARRAY_TYPE,
-      map->elements_kind(), is_store);
+      map->elements_kind(), is_store, store_mode);
 }
 
 
 HInstruction* HOptimizedGraphBuilder::TryBuildConsolidatedElementLoad(
     HValue* object,
     HValue* key,
     HValue* val,
     SmallMapList* maps) {
   // For polymorphic loads of similar elements kinds (i.e. all tagged or all
   // double), always use the "worst case" code without a transition.  This is
@@ skipping 34 matching lines @@
     }
   }
   if (!has_double_maps && !has_smi_or_object_maps) return NULL;
 
   HCheckMaps* check_maps = new(zone()) HCheckMaps(object, maps, zone());
   AddInstruction(check_maps);
   HInstruction* instr = BuildUncheckedMonomorphicElementAccess(
       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;
 }
 
 
 HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess(
     HValue* object,
     HValue* key,
     HValue* val,
     Expression* prop,
     BailoutId ast_id,
     int position,
     bool is_store,
+    KeyedAccessStoreMode store_mode,
     bool* has_side_effects) {
   *has_side_effects = false;
   AddInstruction(new(zone()) HCheckNonSmi(object));
   SmallMapList* maps = prop->GetReceiverTypes();
   bool todo_external_array = false;
 
   if (!is_store) {
     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);
       }
       return consolidated_load;
     }
   }
 
   static const int kNumElementTypes = kElementsKindCount;
   bool type_todo[kNumElementTypes];
@@ skipping 46 matching lines @@
   }
 
   // If only one map is left after transitioning, handle this case
   // monomorphically.
   if (num_untransitionable_maps == 1) {
     HInstruction* instr = NULL;
     if (untransitionable_map->has_slow_elements_kind()) {
       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);
     return is_store ? NULL : instr;
   }
 
   HInstruction* checkspec =
       AddInstruction(HCheckInstanceType::NewIsSpecObject(object, zone()));
   HBasicBlock* join = graph()->CreateBasicBlock();
 
@@ skipping 60 matching lines @@
         typecheck->SetSuccessorAt(1, if_fastobject);
         current_block()->Finish(typecheck);
 
         set_current_block(if_jsarray);
         HInstruction* length;
         length = AddInstruction(new(zone()) HJSArrayLength(object, typecheck,
                                                            HType::Smi()));
         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);
         }
 
         *has_side_effects |= access->HasObservableSideEffects();
         if (position != -1) {
           access->set_position(position);
         }
         if_jsarray->Goto(join);
 
         set_current_block(if_fastobject);
         length = AddInstruction(new(zone()) HFixedArrayBaseLength(elements));
         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));
         } else {
           access = AddInstruction(BuildLoadKeyedGeneric(object, key));
         }
       } else {  // External array elements.
         access = AddInstruction(BuildExternalArrayElementAccess(
             external_elements, checked_key, val,
             elements_kind_branch, elements_kind, is_store));
@@ skipping 25 matching lines @@
     int position,
     bool is_store,
     bool* has_side_effects) {
   ASSERT(!expr->IsPropertyName());
   HInstruction* instr = NULL;
   if (expr->IsMonomorphic()) {
     Handle<Map> map = expr->GetMonomorphicReceiverType();
     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;
 }
 
 
 HInstruction* HOptimizedGraphBuilder::BuildStoreKeyedGeneric(
     HValue* object,
     HValue* key,
     HValue* value) {
   HValue* context = environment()->LookupContext();
@@ skipping 3753 matching lines @@
     }
   }
 
 #ifdef DEBUG
   if (graph_ != NULL) graph_->Verify(false);  // No full verify.
   if (allocator_ != NULL) allocator_->Verify();
 #endif
 }
 
 } }  // namespace v8::internal