Turn ElementsTransitionAndStore stub into a HydrogenCodeStub.

src/code-stubs-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 900 matching lines @@
   }
   return value;
 }
 
 
 Handle<Code> StoreGlobalStub::GenerateCode() {
   return DoGenerateCode(this);
 }
 
 
+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