Use LookupIterator to transition to accessors

BUG=
R=jkummerow@chromium.org

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

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

src/lookup-inl.h

@@ -29,6 +29,7 @@ JSReceiver* LookupIterator::NextHolder(Map* map) {
 
 
 LookupIterator::State LookupIterator::LookupInHolder(Map* map) {
+  STATIC_ASSERT(INTERCEPTOR == BEFORE_PROPERTY);
   DisallowHeapAllocation no_gc;
   switch (state_) {
     case NOT_FOUND:

src/lookup.cc

@@ -5,6 +5,7 @@
 #include "src/v8.h"
 
 #include "src/bootstrapper.h"
+#include "src/deoptimizer.h"
 #include "src/lookup.h"
 #include "src/lookup-inl.h"
 
@@ -109,6 +110,14 @@ bool LookupIterator::HasProperty() {
 }
 
 
+void LookupIterator::ReloadPropertyInformation() {
+  state_ = BEFORE_PROPERTY;
+  state_ = LookupInHolder(*holder_map_);
+  DCHECK(IsFound());
+  HasProperty();
+}
+
+
 void LookupIterator::PrepareForDataProperty(Handle<Object> value) {
   DCHECK(has_property_);
   DCHECK(HolderIsReceiverOrHiddenPrototype());
@@ -116,13 +125,7 @@ void LookupIterator::PrepareForDataProperty(Handle<Object> value) {
   holder_map_ =
       Map::PrepareForDataProperty(holder_map_, descriptor_number(), value);
   JSObject::MigrateToMap(GetHolder<JSObject>(), holder_map_);
-  // Reload property information.
-  if (holder_map_->is_dictionary_map()) {
-    property_encoding_ = DICTIONARY;
-  } else {
-    property_encoding_ = DESCRIPTOR;
-  }
-  CHECK(HasProperty());
+  ReloadPropertyInformation();
 }
 
 
@@ -137,17 +140,12 @@ void LookupIterator::ReconfigureDataProperty(Handle<Object> value,
     JSObject::MigrateToMap(holder, holder_map_);
   }
 
-  // Reload property information and update the descriptor if in dictionary
-  // mode.
   if (holder_map_->is_dictionary_map()) {
-    property_encoding_ = DICTIONARY;
     PropertyDetails details(attributes, NORMAL, 0);
     JSObject::SetNormalizedProperty(holder, name(), value, details);
-  } else {
-    property_encoding_ = DESCRIPTOR;
   }
 
-  CHECK(HasProperty());
+  ReloadPropertyInformation();
 }
 
 
@@ -172,12 +170,62 @@ void LookupIterator::TransitionToDataProperty(
                                               value, attributes, store_mode);
   JSObject::MigrateToMap(receiver, holder_map_);
 
-  // Reload the information.
-  state_ = NOT_FOUND;
-  configuration_ = CHECK_PROPERTY;
-  state_ = LookupInHolder(*holder_map_);
-  DCHECK(IsFound());
-  HasProperty();
+  ReloadPropertyInformation();
+}
+
+
+void LookupIterator::TransitionToAccessorProperty(
+    AccessorComponent component, Handle<Object> accessor,
+    PropertyAttributes attributes) {
+  DCHECK(!accessor->IsNull());
+  // Can only be called when the receiver is a JSObject. JSProxy has to be
+  // handled via a trap. Adding properties to primitive values is not
+  // observable.
+  Handle<JSObject> receiver = Handle<JSObject>::cast(GetReceiver());
+
+  if (receiver->IsJSGlobalProxy()) {
+    PrototypeIterator iter(isolate(), receiver);
+    receiver =
+        Handle<JSGlobalObject>::cast(PrototypeIterator::GetCurrent(iter));
+  }
+
+  maybe_holder_ = receiver;
+  holder_map_ = Map::TransitionToAccessorProperty(
+      handle(receiver->map()), name_, component, accessor, attributes);
+  JSObject::MigrateToMap(receiver, holder_map_);
+
+  ReloadPropertyInformation();
+
+  if (!holder_map_->is_dictionary_map()) return;
+
+  // We have to deoptimize since accesses to data properties may have been
+  // inlined without a corresponding map-check.
+  if (holder_map_->IsGlobalObjectMap()) {
+    Deoptimizer::DeoptimizeGlobalObject(*receiver);
+  }
+
+  // Install the accessor into the dictionary-mode object.
+  PropertyDetails details(attributes, CALLBACKS, 0);
+  Handle<AccessorPair> pair;
+  if (IsFound() && HasProperty() && property_kind() == ACCESSOR &&
+      GetAccessors()->IsAccessorPair()) {
+    pair = Handle<AccessorPair>::cast(GetAccessors());
+    // If the component and attributes are identical, nothing has to be done.
+    if (pair->get(component) == *accessor) {
+      if (property_details().attributes() == attributes) return;
+    } else {
+      pair = AccessorPair::Copy(pair);
+      pair->set(component, *accessor);
+    }
+  } else {
+    pair = isolate()->factory()->NewAccessorPair();
+    pair->set(component, *accessor);
+  }
+  JSObject::SetNormalizedProperty(receiver, name_, pair, details);
+
+  JSObject::ReoptimizeIfPrototype(receiver);
+  holder_map_ = handle(receiver->map());
+  ReloadPropertyInformation();
 }
 
 

src/lookup.h

@@ -31,11 +31,14 @@ class LookupIterator V8_FINAL BASE_EMBEDDED {
   };
 
   enum State {
+    ACCESS_CHECK,
+    INTERCEPTOR,
+    JSPROXY,
     NOT_FOUND,
     PROPERTY,
-    INTERCEPTOR,
-    ACCESS_CHECK,
-    JSPROXY
+    // Set state_ to BEFORE_PROPERTY to ensure that the next lookup will be a
+    // PROPERTY lookup.
+    BEFORE_PROPERTY = INTERCEPTOR
   };
 
   enum PropertyKind {
@@ -100,6 +103,10 @@ class LookupIterator V8_FINAL BASE_EMBEDDED {
 
   bool IsFound() const { return state_ != NOT_FOUND; }
   void Next();
+  void NotFound() {
+    has_property_ = false;
+    state_ = NOT_FOUND;
+  }
 
   Heap* heap() const { return isolate_->heap(); }
   Factory* factory() const { return isolate_->factory(); }
@@ -130,6 +137,9 @@ class LookupIterator V8_FINAL BASE_EMBEDDED {
                                 Object::StoreFromKeyed store_mode);
   void ReconfigureDataProperty(Handle<Object> value,
                                PropertyAttributes attributes);
+  void TransitionToAccessorProperty(AccessorComponent component,
+                                    Handle<Object> accessor,
+                                    PropertyAttributes attributes);
   PropertyKind property_kind() const {
     DCHECK(has_property_);
     return property_kind_;
@@ -162,6 +172,7 @@ class LookupIterator V8_FINAL BASE_EMBEDDED {
   MUST_USE_RESULT inline JSReceiver* NextHolder(Map* map);
   inline State LookupInHolder(Map* map);
   Handle<Object> FetchValue() const;
+  void ReloadPropertyInformation();
 
   bool IsBootstrapping() const;
 

src/objects.h

@@ -2724,22 +2724,6 @@ class JSObject: public JSReceiver {
                                     Handle<Object> getter,
                                     Handle<Object> setter,
                                     PropertyAttributes attributes);
-  static Handle<AccessorPair> CreateAccessorPairFor(Handle<JSObject> object,
-                                                    Handle<Name> name);
-  static void DefinePropertyAccessor(Handle<JSObject> object,
-                                     Handle<Name> name,
-                                     Handle<Object> getter,
-                                     Handle<Object> setter,
-                                     PropertyAttributes attributes);
-
-  // Try to define a single accessor paying attention to map transitions.
-  // Returns false if this was not possible and we have to use the slow case.
-  static bool DefineFastAccessor(Handle<JSObject> object,
-                                 Handle<Name> name,
-                                 AccessorComponent component,
-                                 Handle<Object> accessor,
-                                 PropertyAttributes attributes);
-
 
   // Return the hash table backing store or the inline stored identity hash,
   // whatever is found.
@@ -6462,6 +6446,9 @@ class Map: public HeapObject {
                                               Handle<Object> value,
                                               PropertyAttributes attributes,
                                               StoreFromKeyed store_mode);
+  static Handle<Map> TransitionToAccessorProperty(
+      Handle<Map> map, Handle<Name> name, AccessorComponent component,
+      Handle<Object> accessor, PropertyAttributes attributes);
   static Handle<Map> ReconfigureDataProperty(Handle<Map> map, int descriptor,
                                              PropertyAttributes attributes);
 

src/runtime.cc

@@ -4948,7 +4948,7 @@ static bool IsValidAccessor(Handle<Object> obj) {
 // Transform getter or setter into something DefineAccessor can handle.
 static Handle<Object> InstantiateAccessorComponent(Isolate* isolate,
                                                    Handle<Object> component) {
-  if (component->IsUndefined()) return isolate->factory()->null_value();
+  if (component->IsUndefined()) return isolate->factory()->undefined_value();
   Handle<FunctionTemplateInfo> info =
       Handle<FunctionTemplateInfo>::cast(component);
   return Utils::OpenHandle(*Utils::ToLocal(info)->GetFunction());

src/stub-cache.cc

@@ -1193,18 +1193,6 @@ void ElementHandlerCompiler::GenerateStoreDictionaryElement(
 }
 
 
-CallOptimization::CallOptimization(LookupResult* lookup) {
-  if (lookup->IsFound() &&
-      lookup->IsCacheable() &&
-      lookup->IsConstantFunction()) {
-    // We only optimize constant function calls.
-    Initialize(Handle<JSFunction>(lookup->GetConstantFunction()));
-  } else {
-    Initialize(Handle<JSFunction>::null());
-  }
-}
-
-
 CallOptimization::CallOptimization(Handle<JSFunction> function) {
   Initialize(function);
 }

src/stub-cache.h

@@ -626,8 +626,6 @@ class ElementHandlerCompiler : public PropertyHandlerCompiler {
 // Holds information about possible function call optimizations.
 class CallOptimization BASE_EMBEDDED {
  public:
-  explicit CallOptimization(LookupResult* lookup);
-
   explicit CallOptimization(Handle<JSFunction> function);
 
   bool is_constant_call() const {

src/v8natives.js

@@ -840,8 +840,18 @@ function DefineObjectProperty(obj, p, desc, should_throw) {
     //                 property.
     // Step 12 - updating an existing accessor property with an accessor
     //           descriptor.
-    var getter = desc.hasGetter() ? desc.getGet() : null;
-    var setter = desc.hasSetter() ? desc.getSet() : null;
+    var getter = null;
+    if (desc.hasGetter()) {
+      getter = desc.getGet();
+    } else if (IsAccessorDescriptor(current) && current.hasGetter()) {
+      getter = current.getGet();
+    }
+    var setter = null;
+    if (desc.hasSetter()) {
+      setter = desc.getSet();
+    } else if (IsAccessorDescriptor(current) && current.hasSetter()) {
+      setter = current.getSet();
+    }
     %DefineAccessorPropertyUnchecked(obj, p, getter, setter, flag);
   }
   return true;

test/mjsunit/deopt-global-accessor.js

+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+x = 1;
+x = 2;
+x = 3;
+
+function f() {
+  return x;
+}
+
+f();
+f();
+f();
+%OptimizeFunctionOnNextCall(f);
+f();
+
+Object.defineProperty(this, "x", {get:function() { return 100; }});
+
+assertEquals(100, f());