Turn ElementsTransitionAndStore stub into a HydrogenCodeStub.

src/code-stubs-hydrogen.cc

Index: src/code-stubs-hydrogen.cc
diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc
index 51734da2f16a754110d4acb04f6192e2aeb10664..78a2af2b25fa212d20ee1e4d2c4ee36959cb3a38 100644
--- a/src/code-stubs-hydrogen.cc
+++ b/src/code-stubs-hydrogen.cc
@@ -918,4 +918,94 @@ Handle<Code> StoreGlobalStub::GenerateCode() {
 }
 
 
+template<>
+HValue* CodeStubGraphBuilder<ElementsTransitionAndStoreStub>::BuildCodeStub() {
+  ElementsTransitionAndStoreStub* stub = casted_stub();
+  ElementsKind from_kind = stub->from();
+  ElementsKind to_kind = stub->to();
+
+  HValue* value = GetParameter(0);
+  HValue* target_map = GetParameter(1);
+  HValue* key = GetParameter(2);
+  HValue* array = GetParameter(3);
+
+  ASSERT(!IsFastHoleyElementsKind(from_kind) ||
+         IsFastHoleyElementsKind(to_kind));
+  if (FLAG_trace_elements_transitions) {
+    // Tracing elements transitions is the job of the runtime.
+    current_block()->FinishExitWithDeoptimization(HDeoptimize::kUseAll);
+    set_current_block(NULL);

[danno, 2013/07/10 09:52:37]

I think it's cleaner if you return value here and then the code below doesn't
have to be in an else.

[Benedikt Meurer, 2013/07/10 12:23:09]

Done.

+  } else {
+    if (AllocationSite::GetMode(from_kind, to_kind) == TRACK_ALLOCATION_SITE) {
+      Add<HTrapAllocationMemento>(array);
+    }
+    if (IsFastDoubleElementsKind(from_kind) &&
+        to_kind == FAST_HOLEY_DOUBLE_ELEMENTS) {
+      // Set transitioned map.
+      AddStore(array, HObjectAccess::ForMap(), target_map);

[danno, 2013/07/10 09:52:37]

Why the special casing here? Either the stub should have no side-effects and
defer to the runtime, or it should handle the case completely. Why can you not
just use BuildUncheckedMonomorphicElementAccess with the "holeyified" elements
kind? Also, this case applies to all packed->holey transitions, I think, e.g.
the if should be:

if (GetPackedElementsKind(from_kind) == GetPackedElementsKind(to_kind) &&  
    IsFastPackedElementsKind(from_kind) &&
    IsFastHoleyElementsKind(to_kind)) {
...

[Benedikt Meurer, 2013/07/10 12:23:09]

Done.

+
+      // Let the runtime handle the actual store.
+      current_block()->FinishExitWithDeoptimization(HDeoptimize::kUseAll);
+      set_current_block(NULL);
+    } else {
+      // Check if we need to transition the array elements first
+      // (either SMI -> Double or Double -> Object).
+      if ((IsFastSmiElementsKind(from_kind) &&
+           IsFastDoubleElementsKind(to_kind)) ||
+          (IsFastDoubleElementsKind(from_kind) &&
+           IsFastObjectElementsKind(to_kind))) {

[danno, 2013/07/10 09:52:37]

Can you create a predicate in elements-kind.h to the effect
DoesTransitionChangeElementsBufferFormat or something similar?

[Benedikt Meurer, 2013/07/10 12:23:09]

Done.

+        HInstruction* elements = AddLoadElements(array);
+        HInstruction* elements_length = AddLoadFixedArrayLength(elements);
+
+        IfBuilder if_fixed(this);
+
+        // Check if we have any elements.
+        if_fixed.If<HCompareNumericAndBranch>(elements_length,

[danno, 2013/07/10 09:52:37]

You can use IfNot here and then put the meat of the code below in the "Then" and
omit the "Else"

[Benedikt Meurer, 2013/07/10 12:23:09]

Done.

+                                              graph()->GetConstant0(),
+                                              Token::EQ);
+
+        if_fixed.Then();
+
+        // Nothing to do, just change the map.
+
+        if_fixed.Else();
+
+        // Check if we can allocate in new space.
+        BuildNewSpaceArrayCheck(elements_length, to_kind);

[danno, 2013/07/10 09:52:37]

Why can't you use the helper function "BuildGrowElementsCapacity" for this line
and the following ~10 or so?

[Benedikt Meurer, 2013/07/10 12:23:09]

Done.

[danno, 2013/07/10 12:33:44]

Done.

+
+        // Allocate new elements storage.
+        HValue* new_elements = BuildAllocateElementsAndInitializeElementsHeader(
+            context(), to_kind, elements_length);
+
+        // Convert and copy the elements to new storage.
+        BuildCopyElements(context(),
+                          elements, from_kind,
+                          new_elements, to_kind,
+                          elements_length, elements_length);
+
+        // Set new elements pointer.
+        AddStore(array, HObjectAccess::ForElementsPointer(), new_elements);
+
+        if_fixed.End();
+      }
+
+      // Set transitioned map.
+      AddStore(array, HObjectAccess::ForMap(), target_map);
+
+      // Generate the actual store.
+      BuildUncheckedMonomorphicElementAccess(array, key, value, NULL,

[danno, 2013/07/10 09:52:37]

You can probably factor this out to only be called once for both the elements
format change and simple map changes. For simple map changes, you will have to
first do a 

to_kind = GetHoleyElementsKind(to_kind);

[Benedikt Meurer, 2013/07/10 12:23:09]

Maybe I'm missing the point here, but BuildUncheckedMonomorphicElementAccess is
called only once.

+                                             stub->is_jsarray(), to_kind,
+                                             true, ALLOW_RETURN_HOLE,
+                                             stub->store_mode());
+    }
+  }
+  return value;
+}
+
+
+Handle<Code> ElementsTransitionAndStoreStub::GenerateCode() {
+  return DoGenerateCode(this);
+}
+
+
 } }  // namespace v8::internal