Optimistically assume that elements IC only transition once.

Thanks to Zheng Liu for identifying this issue.

R=jkummerow@chromium.org
BUG=v8:2141
TEST=test/mjsunit/elements-kind.js

Review URL: https://chromiumcodereview.appspot.com/10532063

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11739 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/elements-kind.h

@@ -86,6 +86,17 @@ ElementsKind GetFastElementsKindFromSequenceIndex(int sequence_index);
 int GetSequenceIndexFromFastElementsKind(ElementsKind elements_kind);
 
 
+inline bool IsDictionaryElementsKind(ElementsKind kind) {
+  return kind == DICTIONARY_ELEMENTS;
+}
+
+
+inline bool IsExternalArrayElementsKind(ElementsKind kind) {
+  return kind >= FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND &&
+      kind <= LAST_EXTERNAL_ARRAY_ELEMENTS_KIND;
+}
+
+
 inline bool IsFastElementsKind(ElementsKind kind) {
   ASSERT(FIRST_FAST_ELEMENTS_KIND == 0);
   return kind <= FAST_HOLEY_DOUBLE_ELEMENTS;

src/hydrogen.cc

@@ -5584,7 +5584,7 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
       AddInstruction(transition);
     } else {
       type_todo[map->elements_kind()] = true;
-      if (map->elements_kind() >= FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND) {
+      if (IsExternalArrayElementsKind(map->elements_kind())) {
         todo_external_array = true;
       }
       num_untransitionable_maps++;

src/ic.cc

@@ -1581,38 +1581,43 @@ Handle<Code> KeyedIC::ComputeStub(Handle<JSObject> receiver,
   }
 
   bool monomorphic = false;
+  bool is_transition_stub = IsTransitionStubKind(stub_kind);
+  Handle<Map> receiver_map(receiver->map());
+  Handle<Map> monomorphic_map = receiver_map;
   MapHandleList target_receiver_maps;
-  if (ic_state != UNINITIALIZED && ic_state != PREMONOMORPHIC) {
+  if (ic_state == UNINITIALIZED || ic_state == PREMONOMORPHIC) {
+    // Optimistically assume that ICs that haven't reached the MONOMORPHIC state
+    // yet will do so and stay there.
+    monomorphic = true;
+  } else {
     GetReceiverMapsForStub(Handle<Code>(target()), &target_receiver_maps);
-  }
-  if (!IsTransitionStubKind(stub_kind)) {
-    if (ic_state == UNINITIALIZED || ic_state == PREMONOMORPHIC) {
-      monomorphic = true;
-    } else {
-      if (ic_state == MONOMORPHIC) {
-        // The first time a receiver is seen that is a transitioned version of
-        // the previous monomorphic receiver type, assume the new ElementsKind
-        // is the monomorphic type. This benefits global arrays that only
-        // transition once, and all call sites accessing them are faster if they
-        // remain monomorphic. If this optimistic assumption is not true, the IC
-        // will miss again and it will become polymorphic and support both the
-        // untransitioned and transitioned maps.
-        monomorphic = IsMoreGeneralElementsKindTransition(
-            target_receiver_maps.at(0)->elements_kind(),
-            receiver->GetElementsKind());
-      }
+    if (ic_state == MONOMORPHIC && is_transition_stub) {
+      // The first time a receiver is seen that is a transitioned version of the
+      // previous monomorphic receiver type, assume the new ElementsKind is the
+      // monomorphic type. This benefits global arrays that only transition
+      // once, and all call sites accessing them are faster if they remain
+      // monomorphic. If this optimistic assumption is not true, the IC will
+      // miss again and it will become polymorphic and support both the
+      // untransitioned and transitioned maps.
+      monomorphic = IsMoreGeneralElementsKindTransition(
+          target_receiver_maps.at(0)->elements_kind(),
+          receiver->GetElementsKind());
     }
   }
 
   if (monomorphic) {
+    if (is_transition_stub) {
+      monomorphic_map = ComputeTransitionedMap(receiver, stub_kind);
+      ASSERT(*monomorphic_map != *receiver_map);
+      stub_kind = GetNoTransitionStubKind(stub_kind);
+    }
     return ComputeMonomorphicStub(
-        receiver, stub_kind, strict_mode, generic_stub);
+        monomorphic_map, stub_kind, strict_mode, generic_stub);
   }
   ASSERT(target() != *generic_stub);
 
   // Determine the list of receiver maps that this call site has seen,
   // adding the map that was just encountered.
-  Handle<Map> receiver_map(receiver->map());
   bool map_added =
       AddOneReceiverMapIfMissing(&target_receiver_maps, receiver_map);
   if (IsTransitionStubKind(stub_kind)) {
@@ -1673,16 +1678,16 @@ Handle<Code> KeyedIC::ComputeMonomorphicStubWithoutMapCheck(
 }
 
 
-Handle<Code> KeyedIC::ComputeMonomorphicStub(Handle<JSObject> receiver,
+Handle<Code> KeyedIC::ComputeMonomorphicStub(Handle<Map> receiver_map,
                                              StubKind stub_kind,
                                              StrictModeFlag strict_mode,
                                              Handle<Code> generic_stub) {
-  if (receiver->HasFastSmiOrObjectElements() ||
-      receiver->HasExternalArrayElements() ||
-      receiver->HasFastDoubleElements() ||
-      receiver->HasDictionaryElements()) {
+  ElementsKind elements_kind = receiver_map->elements_kind();
+  if (IsFastElementsKind(elements_kind) ||
+      IsExternalArrayElementsKind(elements_kind) ||
+      IsDictionaryElementsKind(elements_kind)) {
     return isolate()->stub_cache()->ComputeKeyedLoadOrStoreElement(
-        receiver, stub_kind, strict_mode);
+        receiver_map, stub_kind, strict_mode);
   } else {
     return generic_stub;
   }

src/ic.h

@@ -451,7 +451,7 @@ class KeyedIC: public IC {
  private:
   void GetReceiverMapsForStub(Handle<Code> stub, MapHandleList* result);
 
-  Handle<Code> ComputeMonomorphicStub(Handle<JSObject> receiver,
+  Handle<Code> ComputeMonomorphicStub(Handle<Map> receiver_map,
                                       StubKind stub_kind,
                                       StrictModeFlag strict_mode,
                                       Handle<Code> default_stub);
@@ -467,6 +467,12 @@ class KeyedIC: public IC {
   static bool IsGrowStubKind(StubKind stub_kind) {
     return stub_kind >= STORE_AND_GROW_NO_TRANSITION;
   }
+
+  static StubKind GetNoTransitionStubKind(StubKind stub_kind) {
+    if (!IsTransitionStubKind(stub_kind)) return stub_kind;
+    if (IsGrowStubKind(stub_kind)) return STORE_AND_GROW_NO_TRANSITION;
+    return STORE_NO_TRANSITION;
+  }
 };
 
 

src/objects-printer.cc

@@ -559,6 +559,8 @@ void Map::MapPrint(FILE* out) {
   prototype()->ShortPrint(out);
   PrintF(out, "\n - constructor: ");
   constructor()->ShortPrint(out);
+  PrintF(out, "\n - code cache: ");
+  code_cache()->ShortPrint(out);
   PrintF(out, "\n");
 }
 

src/objects.cc

@@ -3302,14 +3302,20 @@ MaybeObject* NormalizedMapCache::Get(JSObject* obj,
       Map::cast(result)->SharedMapVerify();
     }
     if (FLAG_enable_slow_asserts) {
-      // The cached map should match newly created normalized map bit-by-bit.
+      // The cached map should match newly created normalized map bit-by-bit,
+      // except for the code cache, which can contain some ics which can be
+      // applied to the shared map.
       Object* fresh;
       { MaybeObject* maybe_fresh =
             fast->CopyNormalized(mode, SHARED_NORMALIZED_MAP);
         if (maybe_fresh->ToObject(&fresh)) {
           ASSERT(memcmp(Map::cast(fresh)->address(),
                         Map::cast(result)->address(),
-                        Map::kSize) == 0);
+                        Map::kCodeCacheOffset) == 0);
+          int offset = Map::kCodeCacheOffset + kPointerSize;
+          ASSERT(memcmp(Map::cast(fresh)->address() + offset,
+                        Map::cast(result)->address() + offset,
+                        Map::kSize - offset) == 0);
         }
       }
     }
@@ -5040,6 +5046,7 @@ MaybeObject* Map::CopyNormalized(PropertyNormalizationMode mode,
   Map::cast(result)->set_bit_field(bit_field());
   Map::cast(result)->set_bit_field2(bit_field2());
   Map::cast(result)->set_bit_field3(bit_field3());
+  Map::cast(result)->set_code_cache(code_cache());
 
   Map::cast(result)->set_is_shared(sharing == SHARED_NORMALIZED_MAP);
 
@@ -5077,6 +5084,8 @@ void Map::UpdateCodeCache(Handle<Map> map,
 }
 
 MaybeObject* Map::UpdateCodeCache(String* name, Code* code) {
+  ASSERT(!is_shared() || code->allowed_in_shared_map_code_cache());
+
   // Allocate the code cache if not present.
   if (code_cache()->IsFixedArray()) {
     Object* result;
@@ -8403,6 +8412,11 @@ void Code::ClearTypeFeedbackCells(Heap* heap) {
 }
 
 
+bool Code::allowed_in_shared_map_code_cache() {
+  return is_keyed_load_stub() || is_keyed_store_stub();
+}
+
+
 #ifdef ENABLE_DISASSEMBLER
 
 void DeoptimizationInputData::DeoptimizationInputDataPrint(FILE* out) {

src/objects.h

@@ -4327,6 +4327,8 @@ class Code: public HeapObject {
   inline bool has_function_cache();
   inline void set_has_function_cache(bool flag);
 
+  bool allowed_in_shared_map_code_cache();
+
   // Get the safepoint entry for the given pc.
   SafepointEntry GetSafepointEntry(Address pc);
 
@@ -4679,13 +4681,11 @@ class Map: public HeapObject {
   }
 
   inline bool has_external_array_elements() {
-    ElementsKind kind(elements_kind());
-    return kind >= FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND &&
-        kind <= LAST_EXTERNAL_ARRAY_ELEMENTS_KIND;
+    return IsExternalArrayElementsKind(elements_kind());
   }
 
   inline bool has_dictionary_elements() {
-    return elements_kind() == DICTIONARY_ELEMENTS;
+    return IsDictionaryElementsKind(elements_kind());
   }
 
   inline bool has_slow_elements_kind() {

src/stub-cache.cc

@@ -403,7 +403,7 @@ Handle<Code> StubCache::ComputeStoreField(Handle<String> name,
 
 
 Handle<Code> StubCache::ComputeKeyedLoadOrStoreElement(
-    Handle<JSObject> receiver,
+    Handle<Map> receiver_map,
     KeyedIC::StubKind stub_kind,
     StrictModeFlag strict_mode) {
   KeyedAccessGrowMode grow_mode =
@@ -431,7 +431,6 @@ Handle<Code> StubCache::ComputeKeyedLoadOrStoreElement(
       UNREACHABLE();
       break;
   }
-  Handle<Map> receiver_map(receiver->map());
   Handle<Object> probe(receiver_map->FindInCodeCache(*name, flags));
   if (probe->IsCode()) return Handle<Code>::cast(probe);
 
@@ -466,7 +465,7 @@ Handle<Code> StubCache::ComputeKeyedLoadOrStoreElement(
   } else {
     PROFILE(isolate_, CodeCreateEvent(Logger::KEYED_STORE_IC_TAG, *code, 0));
   }
-  JSObject::UpdateMapCodeCache(receiver, name, code);
+  Map::UpdateCodeCache(receiver_map, name, code);
   return code;
 }
 

src/stub-cache.h

@@ -174,7 +174,7 @@ class StubCache {
                                       Handle<Map> transition,
                                       StrictModeFlag strict_mode);
 
-  Handle<Code> ComputeKeyedLoadOrStoreElement(Handle<JSObject> receiver,
+  Handle<Code> ComputeKeyedLoadOrStoreElement(Handle<Map> receiver_map,
                                               KeyedIC::StubKind stub_kind,
                                               StrictModeFlag strict_mode);
 

test/mjsunit/elements-kind.js

@@ -224,9 +224,11 @@ if (support_smi_only_arrays) {
   for (var i = 0; i < 3; i++) {
     convert_mixed(doubles, "three", elements_kind.fast);
   }
+  convert_mixed(construct_smis(), "three", elements_kind.fast);
+  convert_mixed(construct_doubles(), "three", elements_kind.fast);
+  %OptimizeFunctionOnNextCall(convert_mixed);
   smis = construct_smis();
   doubles = construct_doubles();
-  %OptimizeFunctionOnNextCall(convert_mixed);
   convert_mixed(smis, 1, elements_kind.fast);
   convert_mixed(doubles, 1, elements_kind.fast);
   assertTrue(%HaveSameMap(smis, doubles));

test/mjsunit/elements-transition-hoisting.js

@@ -58,6 +58,9 @@ if (support_smi_only_arrays) {
   }
 
   testDoubleConversion4(new Array(5));
+  testDoubleConversion4(new Array(5));  // Call twice to make sure that second
+                                        // store is a transition and not
+                                        // optimistically MONOMORPHIC
   %OptimizeFunctionOnNextCall(testDoubleConversion4);
   testDoubleConversion4(new Array(5));
   testDoubleConversion4(new Array(5));
@@ -73,13 +76,16 @@ if (support_smi_only_arrays) {
     a[1] = 1;
     var count = 3;
     do {
-      a.foo = object; // This map check should be hoistable
+      a.foo = object;  // This map check should be hoistable
       a[1] = object;
       result = a.foo == object && a[1] == object;
     } while (--count > 0);
   }
 
   testExactMapHoisting(new Array(5));
+  testExactMapHoisting(new Array(5));  // Call twice to make sure that second
+                                       // store is a transition and not
+                                       // optimistically MONOMORPHIC
   %OptimizeFunctionOnNextCall(testExactMapHoisting);
   testExactMapHoisting(new Array(5));
   testExactMapHoisting(new Array(5));
@@ -98,15 +104,18 @@ if (support_smi_only_arrays) {
       if (a.bar === undefined) {
         a[1] = 2.5;
       }
-      a.foo = object; // This map check should NOT be hoistable because it
-                      // includes a check for the FAST_ELEMENTS map as well as
-                      // the FAST_DOUBLE_ELEMENTS map, which depends on the
-                      // double transition above in the if, which cannot be
-                      // hoisted.
+      a.foo = object;  // This map check should NOT be hoistable because it
+                       // includes a check for the FAST_ELEMENTS map as well as
+                       // the FAST_DOUBLE_ELEMENTS map, which depends on the
+                       // double transition above in the if, which cannot be
+                       // hoisted.
     } while (--count > 0);
   }
 
   testExactMapHoisting2(new Array(5));
+  testExactMapHoisting2(new Array(5));  // Call twice to make sure that second
+                                        // store is a transition and not
+                                        // optimistically MONOMORPHIC
   %OptimizeFunctionOnNextCall(testExactMapHoisting2);
   testExactMapHoisting2(new Array(5));
   testExactMapHoisting2(new Array(5));
@@ -123,15 +132,18 @@ if (support_smi_only_arrays) {
     var count = 3;
     do {
       a[1] = 2.5;
-      a.foo = object; // This map check should be hoistable because all elements
-                      // transitions in the loop can also be hoisted.
+      a.foo = object;  // This map check should be hoistable because all elements
+                       // transitions in the loop can also be hoisted.
     } while (--count > 0);
   }
 
   var add_transition = new Array(5);
   add_transition.foo = 0;
-  add_transition[0] = new Object(); // For FAST_ELEMENT transition to be created
+  add_transition[0] = new Object();  // For FAST_ELEMENT transition to be created
   testExactMapHoisting3(new Array(5));
+  testExactMapHoisting3(new Array(5));  // Call twice to make sure that second
+                                        // store is a transition and not
+                                        // optimistically MONOMORPHIC
   %OptimizeFunctionOnNextCall(testExactMapHoisting3);
   testExactMapHoisting3(new Array(5));
   testExactMapHoisting3(new Array(5));
@@ -150,6 +162,10 @@ if (support_smi_only_arrays) {
   }
 
   testDominatingTransitionHoisting1(new Array(5));
+  testDominatingTransitionHoisting1(new Array(5));  // Call twice to make sure
+                                                    // that second store is a
+                                                    // transition and not
+                                                    // optimistically MONOMORPHIC
   %OptimizeFunctionOnNextCall(testDominatingTransitionHoisting1);
   testDominatingTransitionHoisting1(new Array(5));
   testDominatingTransitionHoisting1(new Array(5));
@@ -166,6 +182,9 @@ if (support_smi_only_arrays) {
   }
 
   testHoistingWithSideEffect(new Array(5));
+  testHoistingWithSideEffect(new Array(5));  // Call twice to make sure that
+                                             // second store is a transition and
+                                             // not optimistically MONOMORPHIC
   %OptimizeFunctionOnNextCall(testHoistingWithSideEffect);
   testHoistingWithSideEffect(new Array(5));
   testHoistingWithSideEffect(new Array(5));
@@ -179,7 +198,7 @@ if (support_smi_only_arrays) {
       a[1] = c;
       a[2] = d;
       assertTrue(true);
-      a[3] = e; // TransitionElementsKind should be eliminated despite call.
+      a[3] = e;  // TransitionElementsKind should be eliminated despite call.
       a[4] = f;
     } while (--count > 3);
   }