Add a soft-deopt in keyed element access when current IC is pre-monomorphic and no...

BUG=
R=danno@chromium.org

Review URL: https://codereview.chromium.org/32643004

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

diff --git a/src/ast.cc b/src/ast.cc
index 0d667cc168812d0b3f487bf422f1bcf096ac4d1c..481414eb2c2802f0d0922fb6b2f31c8e07361954 100644 (file)
--- a/src/ast.cc
+++ b/src/ast.cc
@@ -139,6 +139,7 @@ Assignment::Assignment(Isolate* isolate,
       assignment_id_(GetNextId(isolate)),
       is_monomorphic_(false),
       is_uninitialized_(false),
+      is_pre_monomorphic_(false),
       store_mode_(STANDARD_STORE) { }
 
 
@@ -426,7 +427,9 @@ void Property::RecordTypeFeedback(TypeFeedbackOracle* oracle,
   is_uninitialized_ = oracle->LoadIsUninitialized(this);
   if (is_uninitialized_) return;
 
+  is_pre_monomorphic_ = oracle->LoadIsPreMonomorphic(this);
   is_monomorphic_ = oracle->LoadIsMonomorphicNormal(this);
+  ASSERT(!is_pre_monomorphic_ || !is_monomorphic_);
   receiver_types_.Clear();
   if (key()->IsPropertyName()) {
     FunctionPrototypeStub proto_stub(Code::LOAD_IC);
@@ -456,7 +459,10 @@ void Assignment::RecordTypeFeedback(TypeFeedbackOracle* oracle,
   TypeFeedbackId id = AssignmentFeedbackId();
   is_uninitialized_ = oracle->StoreIsUninitialized(id);
   if (is_uninitialized_) return;
+
+  is_pre_monomorphic_ = oracle->StoreIsPreMonomorphic(id);
   is_monomorphic_ = oracle->StoreIsMonomorphicNormal(id);
+  ASSERT(!is_pre_monomorphic_ || !is_monomorphic_);
   receiver_types_.Clear();
   if (prop->key()->IsPropertyName()) {
     Literal* lit_key = prop->key()->AsLiteral();

diff --git a/src/ast.h b/src/ast.h
index 4b2010f07773f148a4f416bb5f71031a1e5b20d7..b4f7348eee314eaac011db31d3b238b877d7ef82 100644 (file)
--- a/src/ast.h
+++ b/src/ast.h
@@ -1669,6 +1669,10 @@ class Property V8_FINAL : public Expression {
     return STANDARD_STORE;
   }
   bool IsUninitialized() { return is_uninitialized_; }
+  bool IsPreMonomorphic() { return is_pre_monomorphic_; }
+  bool HasNoTypeInformation() {
+    return is_uninitialized_ || is_pre_monomorphic_;
+  }
   TypeFeedbackId PropertyFeedbackId() { return reuse(id()); }
 
  protected:
@@ -1681,6 +1685,7 @@ class Property V8_FINAL : public Expression {
         key_(key),
         load_id_(GetNextId(isolate)),
         is_monomorphic_(false),
+        is_pre_monomorphic_(false),
         is_uninitialized_(false),
         is_string_access_(false),
         is_function_prototype_(false) { }
@@ -1692,6 +1697,7 @@ class Property V8_FINAL : public Expression {
 
   SmallMapList receiver_types_;
   bool is_monomorphic_ : 1;
+  bool is_pre_monomorphic_ : 1;
   bool is_uninitialized_ : 1;
   bool is_string_access_ : 1;
   bool is_function_prototype_ : 1;
@@ -2098,6 +2104,10 @@ class Assignment V8_FINAL : public Expression {
   void RecordTypeFeedback(TypeFeedbackOracle* oracle, Zone* zone);
   virtual bool IsMonomorphic() V8_OVERRIDE { return is_monomorphic_; }
   bool IsUninitialized() { return is_uninitialized_; }
+  bool IsPreMonomorphic() { return is_pre_monomorphic_; }
+  bool HasNoTypeInformation() {
+    return is_uninitialized_ || is_pre_monomorphic_;
+  }
   virtual SmallMapList* GetReceiverTypes() V8_OVERRIDE {
     return &receiver_types_;
   }
@@ -2130,6 +2140,7 @@ class Assignment V8_FINAL : public Expression {
 
   bool is_monomorphic_ : 1;
   bool is_uninitialized_ : 1;
+  bool is_pre_monomorphic_ : 1;
   KeyedAccessStoreMode store_mode_ : 5;  // Windows treats as signed,
                                          // must have extra bit.
   SmallMapList receiver_types_;

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index da499922a17423059e9473806cd322cf8074c9cd..f46e467fac85727ea00634b30e9c578c589ed9c8 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -5806,13 +5806,14 @@ HValue* HOptimizedGraphBuilder::HandleKeyedElementAccess(
         expr->GetStoreMode(), has_side_effects);
   } else {
     if (is_store) {
-      if (expr->IsAssignment() && expr->AsAssignment()->IsUninitialized()) {
+      if (expr->IsAssignment() &&
+          expr->AsAssignment()->HasNoTypeInformation()) {
         Add<HDeoptimize>("Insufficient type feedback for keyed store",
                          Deoptimizer::SOFT);
       }
       instr = BuildStoreKeyedGeneric(obj, key, val);
     } else {
-      if (expr->AsProperty()->IsUninitialized()) {
+      if (expr->AsProperty()->HasNoTypeInformation()) {
         Add<HDeoptimize>("Insufficient type feedback for keyed load",
                          Deoptimizer::SOFT);
       }

diff --git a/src/ic.cc b/src/ic.cc
index a6ffb13ad40a84b16073b7d6b8a4bbb43ab7e702..4bff543a733305ec33c966e5a890195013e538c5 100644 (file)
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -117,7 +117,9 @@ void IC::TraceIC(const char* type,
 #define TRACE_IC(type, name)             \
   ASSERT((TraceIC(type, name), true))
 
-IC::IC(FrameDepth depth, Isolate* isolate) : isolate_(isolate) {
+IC::IC(FrameDepth depth, Isolate* isolate)
+    : isolate_(isolate),
+      target_set_(false) {
   // To improve the performance of the (much used) IC code, we unfold a few
   // levels of the stack frame iteration code. This yields a ~35% speedup when
   // running DeltaBlue and a ~25% speedup of gbemu with the '--nouse-ic' flag.
@@ -1363,40 +1365,46 @@ MaybeObject* KeyedLoadIC::Load(Handle<Object> object,
     return Runtime::GetObjectPropertyOrFail(isolate(), object, key);
   }
 
+  MaybeObject* maybe_object = NULL;
+  Handle<Code> stub = generic_stub();
+
   // Check for values that can be converted into an internalized string directly
   // or is representable as a smi.
   key = TryConvertKey(key, isolate());
 
   if (key->IsInternalizedString()) {
-    return LoadIC::Load(object, Handle<String>::cast(key));
-  }
-
-  if (FLAG_use_ic && !object->IsAccessCheckNeeded()) {
+    maybe_object = LoadIC::Load(object, Handle<String>::cast(key));
+    if (maybe_object->IsFailure()) return maybe_object;
+  } else if (FLAG_use_ic && !object->IsAccessCheckNeeded()) {
     ASSERT(!object->IsJSGlobalProxy());
-    Handle<Code> stub = generic_stub();
-    if (miss_mode == MISS_FORCE_GENERIC) {
-      TRACE_GENERIC_IC(isolate(), "KeyedLoadIC", "force generic");
-    } else if (object->IsString() && key->IsNumber()) {
-      if (state() == UNINITIALIZED) stub = string_stub();
-    } else if (object->IsJSObject()) {
-      Handle<JSObject> receiver = Handle<JSObject>::cast(object);
-      if (receiver->elements()->map() ==
-          isolate()->heap()->non_strict_arguments_elements_map()) {
-        stub = non_strict_arguments_stub();
-      } else if (receiver->HasIndexedInterceptor()) {
-        stub = indexed_interceptor_stub();
-      } else if (!key->ToSmi()->IsFailure() &&
-                 (!target().is_identical_to(non_strict_arguments_stub()))) {
-        stub = LoadElementStub(receiver);
+    if (miss_mode != MISS_FORCE_GENERIC) {
+      if (object->IsString() && key->IsNumber()) {
+        if (state() == UNINITIALIZED) stub = string_stub();
+      } else if (object->IsJSObject()) {
+        Handle<JSObject> receiver = Handle<JSObject>::cast(object);
+        if (receiver->elements()->map() ==
+            isolate()->heap()->non_strict_arguments_elements_map()) {
+          stub = non_strict_arguments_stub();
+        } else if (receiver->HasIndexedInterceptor()) {
+          stub = indexed_interceptor_stub();
+        } else if (!key->ToSmi()->IsFailure() &&
+                   (!target().is_identical_to(non_strict_arguments_stub()))) {
+          stub = LoadElementStub(receiver);
+        }
       }
     }
+  }
 
+  if (!is_target_set()) {
+    if (*stub == *generic_stub()) {
+      TRACE_GENERIC_IC(isolate(), "KeyedLoadIC", "set generic");
+    }
     ASSERT(!stub.is_null());
     set_target(*stub);
     TRACE_IC("LoadIC", key);
   }
 
-
+  if (maybe_object != NULL) return maybe_object;
   return Runtime::GetObjectPropertyOrFail(isolate(), object, key);
 }
 
@@ -1936,53 +1944,58 @@ MaybeObject* KeyedStoreIC::Store(Handle<Object> object,
   // or is representable as a smi.
   key = TryConvertKey(key, isolate());
 
-  if (key->IsInternalizedString()) {
-    return StoreIC::Store(object,
-                          Handle<String>::cast(key),
-                          value,
-                          JSReceiver::MAY_BE_STORE_FROM_KEYED);
-  }
+  MaybeObject* maybe_object = NULL;
+  Handle<Code> stub = generic_stub();
 
-  bool use_ic = FLAG_use_ic && !object->IsAccessCheckNeeded() &&
-      !(FLAG_harmony_observation && object->IsJSObject() &&
-        JSObject::cast(*object)->map()->is_observed());
-  if (use_ic && !object->IsSmi()) {
-    // Don't use ICs for maps of the objects in Array's prototype chain. We
-    // expect to be able to trap element sets to objects with those maps in the
-    // runtime to enable optimization of element hole access.
-    Handle<HeapObject> heap_object = Handle<HeapObject>::cast(object);
-    if (heap_object->map()->IsMapInArrayPrototypeChain()) use_ic = false;
-  }
-
-  if (use_ic) {
-    ASSERT(!object->IsJSGlobalProxy());
+  if (key->IsInternalizedString()) {
+    maybe_object = StoreIC::Store(object,
+                                  Handle<String>::cast(key),
+                                  value,
+                                  JSReceiver::MAY_BE_STORE_FROM_KEYED);
+    if (maybe_object->IsFailure()) return maybe_object;
+  } else {
+    bool use_ic = FLAG_use_ic && !object->IsAccessCheckNeeded() &&
+        !(FLAG_harmony_observation && object->IsJSObject() &&
+          JSObject::cast(*object)->map()->is_observed());
+    if (use_ic && !object->IsSmi()) {
+      // Don't use ICs for maps of the objects in Array's prototype chain. We
+      // expect to be able to trap element sets to objects with those maps in
+      // the runtime to enable optimization of element hole access.
+      Handle<HeapObject> heap_object = Handle<HeapObject>::cast(object);
+      if (heap_object->map()->IsMapInArrayPrototypeChain()) use_ic = false;
+    }
 
-    Handle<Code> stub = generic_stub();
-    if (miss_mode != MISS_FORCE_GENERIC) {
-      if (object->IsJSObject()) {
-        Handle<JSObject> receiver = Handle<JSObject>::cast(object);
-        bool key_is_smi_like = key->IsSmi() || !key->ToSmi()->IsFailure();
-        if (receiver->elements()->map() ==
-            isolate()->heap()->non_strict_arguments_elements_map()) {
-          stub = non_strict_arguments_stub();
-        } else if (key_is_smi_like &&
-                   (!target().is_identical_to(non_strict_arguments_stub()))) {
-          KeyedAccessStoreMode store_mode = GetStoreMode(receiver, key, value);
-          stub = StoreElementStub(receiver, store_mode);
-        } else {
-          TRACE_GENERIC_IC(isolate(), "KeyedStoreIC", "key not a number");
+    if (use_ic) {
+      ASSERT(!object->IsJSGlobalProxy());
+
+      if (miss_mode != MISS_FORCE_GENERIC) {
+        if (object->IsJSObject()) {
+          Handle<JSObject> receiver = Handle<JSObject>::cast(object);
+          bool key_is_smi_like = key->IsSmi() || !key->ToSmi()->IsFailure();
+          if (receiver->elements()->map() ==
+              isolate()->heap()->non_strict_arguments_elements_map()) {
+            stub = non_strict_arguments_stub();
+          } else if (key_is_smi_like &&
+                     (!target().is_identical_to(non_strict_arguments_stub()))) {
+            KeyedAccessStoreMode store_mode =
+                GetStoreMode(receiver, key, value);
+            stub = StoreElementStub(receiver, store_mode);
+          }
         }
-      } else {
-        TRACE_GENERIC_IC(isolate(), "KeyedStoreIC", "not an object");
       }
-    } else {
-      TRACE_GENERIC_IC(isolate(), "KeyedStoreIC", "force generic");
+    }
+  }
+
+  if (!is_target_set()) {
+    if (*stub == *generic_stub()) {
+      TRACE_GENERIC_IC(isolate(), "KeyedStoreIC", "set generic");
     }
     ASSERT(!stub.is_null());
     set_target(*stub);
     TRACE_IC("StoreIC", key);
   }
 
+  if (maybe_object) return maybe_object;
   return Runtime::SetObjectPropertyOrFail(
       isolate(), object , key, value, NONE, strict_mode());
 }

diff --git a/src/ic.h b/src/ic.h
index 3f0c5c611fdd92a43651040ff56fbf03cbb4a3e4..fde4bc77a58f915c7d2772e75218c15c373eec85 100644 (file)
--- a/src/ic.h
+++ b/src/ic.h
@@ -155,7 +155,12 @@ class IC {
 #endif
 
   // Set the call-site target.
-  void set_target(Code* code) { SetTargetAtAddress(address(), code); }
+  void set_target(Code* code) {
+    SetTargetAtAddress(address(), code);
+    target_set_ = true;
+  }
+
+  bool is_target_set() { return target_set_; }
 
 #ifdef DEBUG
   char TransitionMarkFromState(IC::State state);
@@ -235,6 +240,7 @@ class IC {
   // The original code target that missed.
   Handle<Code> target_;
   State state_;
+  bool target_set_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(IC);
 };

diff --git a/src/type-info.cc b/src/type-info.cc
index da4d1834421520f5dcbe110c433a57fb6c56d362..65d1364058eaf672d99a583480028a37d4da6030 100644 (file)
--- a/src/type-info.cc
+++ b/src/type-info.cc
@@ -128,6 +128,16 @@ bool TypeFeedbackOracle::LoadIsMonomorphicNormal(Property* expr) {
 }
 
 
+bool TypeFeedbackOracle::LoadIsPreMonomorphic(Property* expr) {
+  Handle<Object> map_or_code = GetInfo(expr->PropertyFeedbackId());
+  if (map_or_code->IsCode()) {
+    Handle<Code> code = Handle<Code>::cast(map_or_code);
+    return code->is_inline_cache_stub() && code->ic_state() == PREMONOMORPHIC;
+  }
+  return false;
+}
+
+
 bool TypeFeedbackOracle::LoadIsPolymorphic(Property* expr) {
   Handle<Object> map_or_code = GetInfo(expr->PropertyFeedbackId());
   if (map_or_code->IsCode()) {
@@ -166,6 +176,16 @@ bool TypeFeedbackOracle::StoreIsMonomorphicNormal(TypeFeedbackId ast_id) {
 }
 
 
+bool TypeFeedbackOracle::StoreIsPreMonomorphic(TypeFeedbackId ast_id) {
+  Handle<Object> map_or_code = GetInfo(ast_id);
+  if (map_or_code->IsCode()) {
+    Handle<Code> code = Handle<Code>::cast(map_or_code);
+    return code->ic_state() == PREMONOMORPHIC;
+  }
+  return false;
+}
+
+
 bool TypeFeedbackOracle::StoreIsKeyedPolymorphic(TypeFeedbackId ast_id) {
   Handle<Object> map_or_code = GetInfo(ast_id);
   if (map_or_code->IsCode()) {
@@ -622,12 +642,6 @@ void TypeFeedbackOracle::ProcessRelocInfos(ZoneList<RelocInfo>* infos) {
 
       case Code::KEYED_LOAD_IC:
       case Code::KEYED_STORE_IC:
-        if (target->ic_state() == MONOMORPHIC ||
-            target->ic_state() == POLYMORPHIC) {
-          SetInfo(ast_id, target);
-        }
-        break;
-
       case Code::BINARY_OP_IC:
       case Code::COMPARE_IC:
       case Code::TO_BOOLEAN_IC:

diff --git a/src/type-info.h b/src/type-info.h
index 7d7d7eae4de67c0a97a6a42ddd620f0473d989bd..f295c06dac7feaa52a91a88c031bb755098b3ef3 100644 (file)
--- a/src/type-info.h
+++ b/src/type-info.h
@@ -243,9 +243,11 @@ class TypeFeedbackOracle: public ZoneObject {
 
   bool LoadIsMonomorphicNormal(Property* expr);
   bool LoadIsUninitialized(Property* expr);
+  bool LoadIsPreMonomorphic(Property* expr);
   bool LoadIsPolymorphic(Property* expr);
   bool StoreIsUninitialized(TypeFeedbackId ast_id);
   bool StoreIsMonomorphicNormal(TypeFeedbackId ast_id);
+  bool StoreIsPreMonomorphic(TypeFeedbackId ast_id);
   bool StoreIsKeyedPolymorphic(TypeFeedbackId ast_id);
   bool CallIsMonomorphic(Call* expr);
   bool CallNewIsMonomorphic(CallNew* expr);

diff --git a/test/mjsunit/unbox-double-arrays.js b/test/mjsunit/unbox-double-arrays.js
index 4e8718eb3fe63c6e8f1a68f1a3f1738d16320bd5..5ed404025f69bf08b9ced1c55d973d507590dfa1 100644 (file)
--- a/test/mjsunit/unbox-double-arrays.js
+++ b/test/mjsunit/unbox-double-arrays.js
@@ -345,8 +345,6 @@ function testOneArrayType(allocator) {
                       -Infinity,
                       expected_array_value(7));
 
-  assertOptimized(test_various_stores);
-
   // Make sure that we haven't converted from fast double.
   assertTrue(%HasFastDoubleElements(large_array));
 }