Preparatory refactoring-only steps for inlining accessors.

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 7d0e228e8eeae3e88bc930be8d518966713afa99..7aa2d30e8300f39c9ce9ce041a609a5d00f4733c 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -4764,11 +4764,29 @@ void HGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) {
             property->RecordTypeFeedback(oracle());
             CHECK_ALIVE(VisitForValue(value));
             HValue* value = Pop();
+            Handle<Map> map = property->GetReceiverType();
+            Handle<String> name =
+                Handle<String>::cast(property->key()->handle());
             HInstruction* store;
-            CHECK_ALIVE(store = BuildStoreNamed(literal,
-                                                value,
-                                                property->GetReceiverType(),
-                                                property->key()));
+            if (map.is_null()) {
+              // If we don't know the monomorphic type, do a generic store.
+              CHECK_ALIVE(store = BuildStoreNamedGeneric(literal, name, value));
+            } else {
+              Handle<AccessorPair> accessors;
+              Handle<JSObject> holder;
+              if (IsAccessorCall(map, name, &accessors, &holder)) {
+                CHECK_ALIVE(store = BuildCallSetter(literal,

[Michael Starzinger, 2012/07/19 13:28:13]

For object literals we should never get an accessor because we always use the
internal Object() constructor and duplicate identifiers are not allowed in
literals. So better turn this into an assertion here.

[Sven Panne, 2012/07/23 12:25:01]

Done.

+                                                    value,
+                                                    map,
+                                                    accessors,
+                                                    holder));
+              } else {
+                CHECK_ALIVE(store = BuildStoreNamed(literal,
+                                                    name,
+                                                    value,
+                                                    map));
+              }
+            }
             AddInstruction(store);
             if (store->HasObservableSideEffects()) AddSimulate(key->id());
           } else {
@@ -4939,20 +4957,20 @@ static int ComputeLoadStoreFieldIndex(Handle<Map> type,
 HInstruction* HGraphBuilder::BuildStoreNamedField(HValue* object,
                                                   Handle<String> name,
                                                   HValue* value,
-                                                  Handle<Map> type,
+                                                  Handle<Map> map,
                                                   LookupResult* lookup,
                                                   bool smi_and_map_check) {
   ASSERT(lookup->IsFound());
   if (smi_and_map_check) {
     AddInstruction(new(zone()) HCheckNonSmi(object));
-    AddInstruction(HCheckMaps::NewWithTransitions(object, type, zone()));
+    AddInstruction(HCheckMaps::NewWithTransitions(object, map, zone()));
   }
 
   // If the property does not exist yet, we have to check that it wasn't made
   // readonly or turned into a setter by some meanwhile modifications on the
   // prototype chain.
-  if (!lookup->IsProperty() && type->prototype()->IsJSReceiver()) {
-    Object* proto = type->prototype();
+  if (!lookup->IsProperty() && map->prototype()->IsJSReceiver()) {
+    Object* proto = map->prototype();
     // First check that the prototype chain isn't affected already.
     LookupResult proto_result(isolate());
     proto->Lookup(*name, &proto_result);
@@ -4971,24 +4989,24 @@ HInstruction* HGraphBuilder::BuildStoreNamedField(HValue* object,
     }
     ASSERT(proto->IsJSObject());
     AddInstruction(new(zone()) HCheckPrototypeMaps(
-        Handle<JSObject>(JSObject::cast(type->prototype())),
+        Handle<JSObject>(JSObject::cast(map->prototype())),
         Handle<JSObject>(JSObject::cast(proto))));
   }
 
-  int index = ComputeLoadStoreFieldIndex(type, name, lookup);
+  int index = ComputeLoadStoreFieldIndex(map, name, lookup);
   bool is_in_object = index < 0;
   int offset = index * kPointerSize;
   if (index < 0) {
     // Negative property indices are in-object properties, indexed
     // from the end of the fixed part of the object.
-    offset += type->instance_size();
+    offset += map->instance_size();
   } else {
     offset += FixedArray::kHeaderSize;
   }
   HStoreNamedField* instr =
       new(zone()) HStoreNamedField(object, name, value, is_in_object, offset);
-  if (lookup->IsTransitionToField(*type)) {
-    Handle<Map> transition(lookup->GetTransitionMapFromMap(*type));
+  if (lookup->IsTransitionToField(*map)) {
+    Handle<Map> transition(lookup->GetTransitionMapFromMap(*map));
     instr->set_transition(transition);
     // TODO(fschneider): Record the new map type of the object in the IR to
     // enable elimination of redundant checks after the transition store.
@@ -5025,53 +5043,28 @@ static void LookupInPrototypes(Handle<Map> map,
 }
 
 
-HInstruction* HGraphBuilder::BuildCallSetter(HValue* obj,
-                                             Handle<String> name,
+HInstruction* HGraphBuilder::BuildCallSetter(HValue* object,
                                              HValue* value,
                                              Handle<Map> map,
-                                             Handle<Object> callback,
+                                             Handle<AccessorPair> accessors,
                                              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));
+  Handle<JSFunction> function(JSFunction::cast(accessors->setter()));
+  AddCheckConstantFunction(holder, object, map, true);
+  AddInstruction(new(zone()) HPushArgument(object));
   AddInstruction(new(zone()) HPushArgument(value));
   return new(zone()) HCallConstantFunction(function, 2);
 }
 
 
 HInstruction* HGraphBuilder::BuildStoreNamed(HValue* object,
+                                             Handle<String> name,
                                              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());
-  if (type.is_null()) return BuildStoreNamedGeneric(object, name, value);
-
+                                             Handle<Map> map) {
   // Handle a store to a known field.
   LookupResult lookup(isolate());
-  if (ComputeLoadStoreField(type, name, &lookup, true)) {
+  if (ComputeLoadStoreField(map, 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 BuildStoreNamedField(object, name, value, map, &lookup, true);
   }
 
   // No luck, do a generic store.
@@ -5118,7 +5111,7 @@ void HGraphBuilder::HandlePolymorphicLoadNamedField(Property* expr,
   HInstruction* instr;
   if (count == types->length() && is_monomorphic_field) {
     AddInstruction(new(zone()) HCheckMaps(object, types, zone()));
-    instr = BuildLoadNamedField(object, expr, map, &lookup, false);
+    instr = BuildLoadNamedField(object, map, &lookup, false);
   } else {
     HValue* context = environment()->LookupContext();
     instr = new(zone()) HLoadNamedFieldPolymorphic(context,
@@ -5230,10 +5223,21 @@ void HGraphBuilder::HandlePropertyAssignment(Assignment* expr) {
 
     SmallMapList* types = expr->GetReceiverTypes();
     if (expr->IsMonomorphic()) {
-      CHECK_ALIVE(instr = BuildStoreNamed(object,
-                                          value,
-                                          types->first(),
-                                          prop->key()));
+      Handle<Map> type = types->first();
+      Handle<AccessorPair> accessors;
+      Handle<JSObject> holder;
+      if (IsAccessorCall(type, name, &accessors, &holder)) {
+        CHECK_ALIVE(instr = BuildCallSetter(object,
+                                            value,
+                                            type,
+                                            accessors,
+                                            holder));
+      } else {
+        CHECK_ALIVE(instr = BuildStoreNamed(object,
+                                            name,
+                                            value,
+                                            type));
+      }
 
     } else if (types != NULL && types->length() > 1) {
       HandlePolymorphicStoreNamedField(expr, object, value, types, name);
@@ -5391,16 +5395,22 @@ void HGraphBuilder::HandleCompoundAssignment(Assignment* expr) {
     if (prop->key()->IsPropertyName()) {
       // Named property.
       CHECK_ALIVE(VisitForValue(prop->obj()));
-      HValue* obj = Top();
+      HValue* object = Top();
 
+      Handle<String> name = prop->key()->AsLiteral()->AsPropertyName();
       Handle<Map> map;
       HInstruction* load;
       if (prop->IsMonomorphic()) {
-        Handle<String> name = prop->key()->AsLiteral()->AsPropertyName();
         map = prop->GetReceiverTypes()->first();
-        load = BuildLoadNamed(obj, prop, map, name);
+        Handle<AccessorPair> accessors;
+        Handle<JSObject> holder;
+        if (IsAccessorCall(map, name, &accessors, &holder)) {
+          load = BuildCallGetter(object, map, accessors, holder);
+        } else {
+          load = BuildLoadNamed(object, name, prop, map);
+        }

[Michael Starzinger, 2012/07/19 13:28:13]

This part cannot yet be inlined with out current machinery, because it requires
follow-up work after the inlined getter returns. Unfortunately the getter will
be inlined into the outer AST-context, which might be a test-context. In this
case the is no single basic block that can contain the follow-up work.

What we actually want to do is inline the getter into an artificial
value-context (similar to what VisitForValue does), so that we have a single
return block.

       } else {
-        load = BuildLoadNamedGeneric(obj, prop);
+        load = BuildLoadNamedGeneric(object, name, prop);
       }
       PushAndAdd(load);
       if (load->HasObservableSideEffects()) AddSimulate(expr->CompoundLoadId());
@@ -5414,7 +5424,22 @@ void HGraphBuilder::HandleCompoundAssignment(Assignment* expr) {
       if (instr->HasObservableSideEffects()) AddSimulate(operation->id());
 
       HInstruction* store;
-      CHECK_ALIVE(store = BuildStoreNamed(obj, instr, map, prop->key()));
+      if (map.is_null()) {
+        // If we don't know the monomorphic type, do a generic store.
+        CHECK_ALIVE(store = BuildStoreNamedGeneric(object, name, instr));
+      } else {
+        Handle<AccessorPair> accessors;
+        Handle<JSObject> holder;
+        if (IsAccessorCall(map, name, &accessors, &holder)) {
+          CHECK_ALIVE(store = BuildCallSetter(object,
+                                              instr,
+                                              map,
+                                              accessors,
+                                              holder));
+        } else {
+          CHECK_ALIVE(store = BuildStoreNamed(object, name, instr, map));
+        }
+      }
       AddInstruction(store);
       // Drop the simulated receiver and value.  Return the value.
       Drop(2);
@@ -5615,20 +5640,19 @@ void HGraphBuilder::VisitThrow(Throw* expr) {
 
 
 HLoadNamedField* HGraphBuilder::BuildLoadNamedField(HValue* object,
-                                                    Property* expr,
-                                                    Handle<Map> type,
+                                                    Handle<Map> map,
                                                     LookupResult* lookup,
                                                     bool smi_and_map_check) {
   if (smi_and_map_check) {
     AddInstruction(new(zone()) HCheckNonSmi(object));
-    AddInstruction(HCheckMaps::NewWithTransitions(object, type, zone()));
+    AddInstruction(HCheckMaps::NewWithTransitions(object, map, zone()));
   }
 
-  int index = lookup->GetLocalFieldIndexFromMap(*type);
+  int index = lookup->GetLocalFieldIndexFromMap(*map);
   if (index < 0) {
     // Negative property indices are in-object properties, indexed
     // from the end of the fixed part of the object.
-    int offset = (index * kPointerSize) + type->instance_size();
+    int offset = (index * kPointerSize) + map->instance_size();
     return new(zone()) HLoadNamedField(object, true, offset);
   } else {
     // Non-negative property indices are in the properties array.
@@ -5638,63 +5662,79 @@ HLoadNamedField* HGraphBuilder::BuildLoadNamedField(HValue* object,
 }
 
 
-HInstruction* HGraphBuilder::BuildLoadNamedGeneric(HValue* obj,
+HInstruction* HGraphBuilder::BuildLoadNamedGeneric(HValue* object,
+                                                   Handle<String> name,
                                                    Property* expr) {
   if (expr->IsUninitialized() && !FLAG_always_opt) {
     AddInstruction(new(zone()) HSoftDeoptimize);
     current_block()->MarkAsDeoptimizing();
   }
-  ASSERT(expr->key()->IsPropertyName());
-  Handle<Object> name = expr->key()->AsLiteral()->handle();
   HValue* context = environment()->LookupContext();
-  return new(zone()) HLoadNamedGeneric(context, obj, name);
+  return new(zone()) HLoadNamedGeneric(context, object, name);
 }
 
 
-HInstruction* HGraphBuilder::BuildCallGetter(HValue* obj,
-                                             Property* expr,
+HInstruction* HGraphBuilder::BuildCallGetter(HValue* object,
                                              Handle<Map> map,
-                                             Handle<Object> callback,
+                                             Handle<AccessorPair> accessors,
                                              Handle<JSObject> holder) {
-  if (!callback->IsAccessorPair()) return BuildLoadNamedGeneric(obj, expr);
-  Handle<Object> getter(Handle<AccessorPair>::cast(callback)->getter());
-  Handle<JSFunction> function(Handle<JSFunction>::cast(getter));
-  AddCheckConstantFunction(holder, obj, map, true);
-  AddInstruction(new(zone()) HPushArgument(obj));
+  Handle<JSFunction> function(JSFunction::cast(accessors->getter()));
+  AddCheckConstantFunction(holder, object, map, true);
+  AddInstruction(new(zone()) HPushArgument(object));
   return new(zone()) HCallConstantFunction(function, 1);
 }
 
 
-HInstruction* HGraphBuilder::BuildLoadNamed(HValue* obj,
+bool HGraphBuilder::IsAccessorCall(Handle<Map> map,

[Michael Starzinger, 2012/07/19 13:28:13]

The name of this method is misleading, because it actually performs a lookup.
Can we call it "LookupAccessorCall" or "LookupAccessorPair" instead?

[Sven Panne, 2012/07/23 12:25:01]

Done.

+                                   Handle<String> name,
+                                   Handle<AccessorPair>* accessors,
+                                   Handle<JSObject>* holder) {
+  LookupResult lookup(isolate());
+
+  // Check for a JavaScript accessor directly in the map.
+  map->LookupDescriptor(NULL, *name, &lookup);
+  if (lookup.IsPropertyCallbacks()) {
+    Handle<Object> callback(lookup.GetValueFromMap(*map));
+    if (!callback->IsAccessorPair()) return false;
+    *accessors = Handle<AccessorPair>::cast(callback);
+    *holder = Handle<JSObject>();
+    return true;
+  }
+
+  // Everything else, e.g. a field, can't be an accessor call.
+  if (lookup.IsFound()) return false;
+
+  // Check for a JavaScript accessor somewhere in the proto chain.
+  LookupInPrototypes(map, name, &lookup);
+  if (lookup.IsPropertyCallbacks()) {
+    Handle<Object> callback(lookup.GetValue());
+    if (!callback->IsAccessorPair()) return false;
+    *accessors = Handle<AccessorPair>::cast(callback);
+    *holder = Handle<JSObject>(lookup.holder());
+    return true;
+  }
+
+  // We haven't found a JavaScript accessor anywhere.
+  return false;
+}
+
+
+HInstruction* HGraphBuilder::BuildLoadNamed(HValue* object,
+                                            Handle<String> name,
                                             Property* expr,
-                                            Handle<Map> map,
-                                            Handle<String> name) {
+                                            Handle<Map> map) {
   LookupResult lookup(isolate());
   map->LookupDescriptor(NULL, *name, &lookup);
   if (lookup.IsField()) {
-    return BuildLoadNamedField(obj,
-                               expr,
-                               map,
-                               &lookup,
-                               true);
-  } else if (lookup.IsConstantFunction()) {
-    AddInstruction(new(zone()) HCheckNonSmi(obj));
-    AddInstruction(HCheckMaps::NewWithTransitions(obj, map, zone()));
+    return BuildLoadNamedField(object, map, &lookup, true);
+  }
+  if (lookup.IsConstantFunction()) {
+    AddInstruction(new(zone()) HCheckNonSmi(object));
+    AddInstruction(HCheckMaps::NewWithTransitions(object, map, zone()));
     Handle<JSFunction> function(lookup.GetConstantFunctionFromMap(*map));
     return new(zone()) HConstant(function, Representation::Tagged());
-  } else if (lookup.IsPropertyCallbacks()) {
-    Handle<Object> callback(lookup.GetValueFromMap(*map));
-    Handle<JSObject> holder;
-    return BuildCallGetter(obj, expr, map, callback, holder);
-  } else {
-    LookupInPrototypes(map, name, &lookup);
-    if (lookup.IsPropertyCallbacks()) {
-      Handle<Object> callback(lookup.GetValue());
-      Handle<JSObject> holder(lookup.holder());
-      return BuildCallGetter(obj, expr, map, callback, holder);
-    }
-    return BuildLoadNamedGeneric(obj, expr);
   }
+  return BuildLoadNamedGeneric(object, name, expr);
 }
 
 
@@ -6301,26 +6341,33 @@ void HGraphBuilder::VisitProperty(Property* expr) {
     Handle<String> name = expr->key()->AsLiteral()->AsPropertyName();
     SmallMapList* types = expr->GetReceiverTypes();
 
-    HValue* obj = Pop();
+    HValue* object = Pop();
     if (expr->IsMonomorphic()) {
-      instr = BuildLoadNamed(obj, expr, types->first(), name);
+      Handle<Map> map = types->first();
+      Handle<AccessorPair> accessors;
+      Handle<JSObject> holder;
+      if (IsAccessorCall(map, name, &accessors, &holder)) {
+        instr = BuildCallGetter(object, map, accessors, holder);
+      } else {
+        instr = BuildLoadNamed(object, name, expr, map);
+      }
     } else if (types != NULL && types->length() > 1) {
-      AddInstruction(new(zone()) HCheckNonSmi(obj));
-      HandlePolymorphicLoadNamedField(expr, obj, types, name);
+      AddInstruction(new(zone()) HCheckNonSmi(object));
+      HandlePolymorphicLoadNamedField(expr, object, types, name);
       return;
     } else {
-      instr = BuildLoadNamedGeneric(obj, expr);
+      instr = BuildLoadNamedGeneric(object, name, expr);
     }
 
   } else {
     CHECK_ALIVE(VisitForValue(expr->key()));
 
     HValue* key = Pop();
-    HValue* obj = Pop();
+    HValue* object = Pop();
 
     bool has_side_effects = false;
     HValue* load = HandleKeyedElementAccess(
-        obj, key, NULL, expr, expr->id(), expr->position(),
+        object, key, NULL, expr, expr->id(), expr->position(),
         false,  // is_store
         &has_side_effects);
     if (has_side_effects) {
@@ -7788,14 +7835,20 @@ void HGraphBuilder::VisitCountOperation(CountOperation* expr) {
       CHECK_ALIVE(VisitForValue(prop->obj()));
       HValue* obj = Top();
 
+      Handle<String> name = prop->key()->AsLiteral()->AsPropertyName();
       Handle<Map> map;
       HInstruction* load;
       if (prop->IsMonomorphic()) {
-        Handle<String> name = prop->key()->AsLiteral()->AsPropertyName();
         map = prop->GetReceiverTypes()->first();
-        load = BuildLoadNamed(obj, prop, map, name);
+        Handle<AccessorPair> accessors;
+        Handle<JSObject> holder;
+        if (IsAccessorCall(map, name, &accessors, &holder)) {
+          load = BuildCallGetter(obj, map, accessors, holder);
+        } else {
+          load = BuildLoadNamed(obj, name, prop, map);
+        }

[Michael Starzinger, 2012/07/19 13:28:13]

Same comment as for HandleCompoundAssignment() applies here.

       } else {
-        load = BuildLoadNamedGeneric(obj, prop);
+        load = BuildLoadNamedGeneric(obj, name, prop);
       }
       PushAndAdd(load);
       if (load->HasObservableSideEffects()) AddSimulate(expr->CountId());
@@ -7804,7 +7857,22 @@ void HGraphBuilder::VisitCountOperation(CountOperation* expr) {
       input = Pop();
 
       HInstruction* store;
-      CHECK_ALIVE(store = BuildStoreNamed(obj, after, map, prop->key()));
+      if (map.is_null()) {
+        // If we don't know the monomorphic type, do a generic store.
+        CHECK_ALIVE(store = BuildStoreNamedGeneric(obj, name, after));
+      } else {
+        Handle<AccessorPair> accessors;
+        Handle<JSObject> holder;
+        if (IsAccessorCall(map, name, &accessors, &holder)) {
+          CHECK_ALIVE(store = BuildCallSetter(obj,
+                                              after,
+                                              map,
+                                              accessors,
+                                              holder));
+        } else {
+          CHECK_ALIVE(store = BuildStoreNamed(obj, name, after, map));
+        }
+      }
       AddInstruction(store);
 
       // Overwrite the receiver in the bailout environment with the result