Added Crankshaft support for setters.
authorsvenpanne@chromium.org <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 13 Jul 2012 07:14:28 +0000 (07:14 +0000)
committersvenpanne@chromium.org <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 13 Jul 2012 07:14:28 +0000 (07:14 +0000)
Refactored ComputeLoadStoreField a bit on the way to clarify a bit what it
actually does.

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

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

src/hydrogen.cc
src/hydrogen.h
test/mjsunit/regress/regress-crbug-125148.js

index 42d0fd8..0897aad 100644 (file)
@@ -4911,11 +4911,18 @@ static bool ComputeLoadStoreField(Handle<Map> type,
                                   Handle<String> name,
                                   LookupResult* lookup,
                                   bool is_store) {
-  type->LookupTransitionOrDescriptor(NULL, *name, lookup);
+  // If we directly find a field, the access can be inlined.
+  type->LookupDescriptor(NULL, *name, lookup);
   if (lookup->IsField()) return true;
-  return is_store &&
-         lookup->IsTransitionToField(*type) &&
-         (type->unused_property_fields() > 0);
+
+  // For a load, we are out of luck if there is no such field.
+  if (!is_store) return false;
+
+  // 2nd chance: A store into a non-existent field can still be inlined if we
+  // have a matching transition and some room left in the object.
+  type->LookupTransition(NULL, *name, lookup);
+  return lookup->IsTransitionToField(*type) &&
+      (type->unused_property_fields() > 0);
 }
 
 
@@ -5007,21 +5014,71 @@ HInstruction* HGraphBuilder::BuildStoreNamedGeneric(HValue* object,
 }
 
 
+static void LookupInPrototypes(Handle<Map> map,
+                               Handle<String> name,
+                               LookupResult* lookup) {
+  while (map->prototype()->IsJSObject()) {
+    Handle<JSObject> holder(JSObject::cast(map->prototype()));
+    if (!holder->HasFastProperties()) break;
+    map = Handle<Map>(holder->map());
+    map->LookupDescriptor(*holder, *name, lookup);
+    if (lookup->IsFound()) return;
+  }
+  lookup->NotFound();
+}
+
+
+HInstruction* HGraphBuilder::BuildCallSetter(HValue* obj,
+                                             Handle<String> name,
+                                             HValue* value,
+                                             Handle<Map> map,
+                                             Handle<Object> callback,
+                                             Handle<JSObject> holder) {
+  if (!callback->IsAccessorPair()) {
+    return BuildStoreNamedGeneric(obj, name, value);
+  }
+  Handle<Object> setter(Handle<AccessorPair>::cast(callback)->setter());
+  Handle<JSFunction> function(Handle<JSFunction>::cast(setter));
+  AddCheckConstantFunction(holder, obj, map, true);
+  AddInstruction(new(zone()) HPushArgument(obj));
+  AddInstruction(new(zone()) HPushArgument(value));
+  return new(zone()) HCallConstantFunction(function, 2);
+}
+
+
 HInstruction* HGraphBuilder::BuildStoreNamed(HValue* object,
                                              HValue* value,
                                              Handle<Map> type,
                                              Expression* key) {
+  // If we don't know the monomorphic type, do a generic store.
   Handle<String> name = Handle<String>::cast(key->AsLiteral()->handle());
-  ASSERT(!name.is_null());
+  if (type.is_null()) return BuildStoreNamedGeneric(object, name, value);
 
+  // Handle a store to a known field.
   LookupResult lookup(isolate());
-  bool is_monomorphic = !type.is_null() &&
-      ComputeLoadStoreField(type, name, &lookup, true);
+  if (ComputeLoadStoreField(type, name, &lookup, true)) {
+    // true = needs smi and map check.
+    return BuildStoreNamedField(object, name, value, type, &lookup, true);
+  }
+
+  // Handle a known setter directly in the receiver.
+  type->LookupDescriptor(NULL, *name, &lookup);
+  if (lookup.IsPropertyCallbacks()) {
+    Handle<Object> callback(lookup.GetValueFromMap(*type));
+    Handle<JSObject> holder;
+    return BuildCallSetter(object, name, value, type, callback, holder);
+  }
+
+  // Handle a known setter somewhere in the prototype chain.
+  LookupInPrototypes(type, name, &lookup);
+  if (lookup.IsPropertyCallbacks()) {
+    Handle<Object> callback(lookup.GetValue());
+    Handle<JSObject> holder(lookup.holder());
+    return BuildCallSetter(object, name, value, type, callback, holder);
+  }
 
-  return is_monomorphic
-      ? BuildStoreNamedField(object, name, value, type, &lookup,
-                             true)  // Needs smi and map check.
-      : BuildStoreNamedGeneric(object, name, value);
+  // No luck, do a generic store.
+  return BuildStoreNamedGeneric(object, name, value);
 }
 
 
@@ -5597,20 +5654,6 @@ HInstruction* HGraphBuilder::BuildLoadNamedGeneric(HValue* obj,
 }
 
 
-static void LookupInPrototypes(Handle<Map> map,
-                               Handle<String> name,
-                               LookupResult* lookup) {
-  while (map->prototype()->IsJSObject()) {
-    Handle<JSObject> holder(JSObject::cast(map->prototype()));
-    if (!holder->HasFastProperties()) break;
-    map = Handle<Map>(holder->map());
-    map->LookupDescriptor(*holder, *name, lookup);
-    if (lookup->IsFound()) return;
-  }
-  lookup->NotFound();
-}
-
-
 HInstruction* HGraphBuilder::BuildCallGetter(HValue* obj,
                                              Property* expr,
                                              Handle<Map> map,
index c47b7f2..9aac71c 100644 (file)
@@ -1149,6 +1149,12 @@ class HGraphBuilder: public AstVisitor {
                                Property* prop,
                                Handle<Map> map,
                                Handle<String> name);
+  HInstruction* BuildCallSetter(HValue* object,
+                                Handle<String> name,
+                                HValue* value,
+                                Handle<Map> map,
+                                Handle<Object> callback,
+                                Handle<JSObject> holder);
   HInstruction* BuildStoreNamed(HValue* object,
                                 HValue* value,
                                 Handle<Map> type,
index 142f140..0f7bcd8 100644 (file)
@@ -66,3 +66,25 @@ assertEquals(111, boo(C));
 ToDictionaryMode(B);
 %OptimizeFunctionOnNextCall(boo);
 assertEquals(111, boo(C));
+
+// And once more for setters...
+A = {};
+Object.defineProperty(A, "huh", { set: function(x) { assertUnreachable(); }});
+
+B = Object.create(A);
+var setterValue;
+Object.defineProperty(B, "huh", { set: function(x) { setterValue = x; }});
+
+C = Object.create(B);
+
+function fuu(x) {
+  setterValue = 222;
+  x.huh = 111;
+  return setterValue;
+}
+
+assertEquals(111, fuu(C));
+assertEquals(111, fuu(C));
+ToDictionaryMode(B);
+%OptimizeFunctionOnNextCall(fuu);
+assertEquals(111, fuu(C));