Let BuildStore/BuildLoad distinguish between keyed/named load/stores.

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index bd9b0d69967ecec4e3e3b2a99afee473d81b30ce..3cbe1cb392751a97f5568d110a9c2140cbe7f73a 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -4896,109 +4896,51 @@ void HOptimizedGraphBuilder::HandlePolymorphicStoreNamedField(
 }
 
 
-void HOptimizedGraphBuilder::HandlePropertyAssignment(Assignment* expr) {
-  Property* prop = expr->target()->AsProperty();
-  ASSERT(prop != NULL);
-  CHECK_ALIVE(VisitForValue(prop->obj()));
+static bool ComputeReceiverTypes(Expression* expr,
+                                 HValue* receiver,
+                                 SmallMapList** t) {
+  SmallMapList* types = expr->GetReceiverTypes();
+  *t = types;
+  bool monomorphic = expr->IsMonomorphic();
+  if (types != NULL && receiver->HasMonomorphicJSObjectType()) {
+    Map* root_map = receiver->GetMonomorphicJSObjectMap()->FindRootMap();
+    types->FilterForPossibleTransitions(root_map);
+    monomorphic = types->length() == 1;
+  }
+  return monomorphic && CanInlinePropertyAccess(*types->first());
+}
 
-  if (prop->key()->IsPropertyName()) {
-    // Named store.
-    CHECK_ALIVE(VisitForValue(expr->value()));
-    HValue* value = environment()->ExpressionStackAt(0);
-    HValue* object = environment()->ExpressionStackAt(1);
 
-    if (expr->IsUninitialized()) {
-      Add<HDeoptimize>("Insufficient type feedback for property assignment",
-                       Deoptimizer::SOFT);
-    }
-    return BuildStoreNamed(
-        expr, expr->id(), expr->AssignmentId(), prop, object, value);
-  } else {
+void HOptimizedGraphBuilder::BuildStore(Expression* expr,
+                                        Property* prop,
+                                        BailoutId ast_id,
+                                        BailoutId return_id,
+                                        bool is_uninitialized) {
+  HValue* value = environment()->ExpressionStackAt(0);
+
+  if (!prop->key()->IsPropertyName()) {
     // Keyed store.
-    CHECK_ALIVE(VisitForValue(prop->key()));
-    CHECK_ALIVE(VisitForValue(expr->value()));
-    HValue* value = environment()->ExpressionStackAt(0);
     HValue* key = environment()->ExpressionStackAt(1);
     HValue* object = environment()->ExpressionStackAt(2);
     bool has_side_effects = false;
-    HandleKeyedElementAccess(object, key, value, expr, expr->AssignmentId(),
+    HandleKeyedElementAccess(object, key, value, expr, return_id,
                              expr->position(),
                              true,  // is_store
                              &has_side_effects);
     Drop(3);
     Push(value);
-    Add<HSimulate>(expr->AssignmentId(), REMOVABLE_SIMULATE);
+    Add<HSimulate>(return_id, REMOVABLE_SIMULATE);
     return ast_context()->ReturnValue(Pop());
   }
-}
-
-
-// Because not every expression has a position and there is not common
-// superclass of Assignment and CountOperation, we cannot just pass the
-// owning expression instead of position and ast_id separately.
-void HOptimizedGraphBuilder::HandleGlobalVariableAssignment(
-    Variable* var,
-    HValue* value,
-    int position,
-    BailoutId ast_id) {
-  LookupResult lookup(isolate());
-  GlobalPropertyAccess type = LookupGlobalProperty(var, &lookup, true);
-  if (type == kUseCell) {
-    Handle<GlobalObject> global(current_info()->global_object());
-    Handle<PropertyCell> cell(global->GetPropertyCell(&lookup));
-    if (cell->type()->IsConstant()) {
-      IfBuilder builder(this);
-      HValue* constant = Add<HConstant>(cell->type()->AsConstant());
-      if (cell->type()->AsConstant()->IsNumber()) {
-        builder.If<HCompareNumericAndBranch>(value, constant, Token::EQ);
-      } else {
-        builder.If<HCompareObjectEqAndBranch>(value, constant);
-      }
-      builder.Then();
-      builder.Else();
-      Add<HDeoptimize>("Constant global variable assignment",
-                       Deoptimizer::EAGER);
-      builder.End();
-    }
-    HInstruction* instr =
-        Add<HStoreGlobalCell>(value, cell, lookup.GetPropertyDetails());
-    instr->set_position(position);
-    if (instr->HasObservableSideEffects()) {
-      Add<HSimulate>(ast_id, REMOVABLE_SIMULATE);
-    }
-  } else {
-    HGlobalObject* global_object = Add<HGlobalObject>();
-    HStoreGlobalGeneric* instr =
-        Add<HStoreGlobalGeneric>(global_object, var->name(),
-                                 value, function_strict_mode_flag());
-    instr->set_position(position);
-    ASSERT(instr->HasObservableSideEffects());
-    Add<HSimulate>(ast_id, REMOVABLE_SIMULATE);
-  }
-}
 
+  // Named store.
+  HValue* object = environment()->ExpressionStackAt(1);
 
-static bool ComputeReceiverTypes(Expression* expr,
-                                 HValue* receiver,
-                                 SmallMapList** t) {
-  SmallMapList* types = expr->GetReceiverTypes();
-  *t = types;
-  bool monomorphic = expr->IsMonomorphic();
-  if (types != NULL && receiver->HasMonomorphicJSObjectType()) {
-    Map* root_map = receiver->GetMonomorphicJSObjectMap()->FindRootMap();
-    types->FilterForPossibleTransitions(root_map);
-    monomorphic = types->length() == 1;
+  if (is_uninitialized) {
+    Add<HDeoptimize>("Insufficient type feedback for property assignment",
+                     Deoptimizer::SOFT);
   }
-  return monomorphic && CanInlinePropertyAccess(*types->first());
-}
-
 
-void HOptimizedGraphBuilder::BuildStoreNamed(Expression* expr,
-                                             BailoutId id,
-                                             BailoutId assignment_id,
-                                             Property* prop,
-                                             HValue* object,
-                                             HValue* value) {
   Literal* key = prop->key()->AsLiteral();
   Handle<String> name = Handle<String>::cast(key->value());
   ASSERT(!name.is_null());
@@ -5015,7 +4957,7 @@ void HOptimizedGraphBuilder::BuildStoreNamed(Expression* expr,
     if (LookupSetter(map, name, &setter, &holder)) {
       AddCheckConstantFunction(holder, object, map);
       if (FLAG_inline_accessors &&
-          TryInlineSetter(setter, id, assignment_id, value)) {
+          TryInlineSetter(setter, ast_id, return_id, value)) {
         return;
       }
       Drop(2);
@@ -5032,7 +4974,7 @@ void HOptimizedGraphBuilder::BuildStoreNamed(Expression* expr,
   } else if (types != NULL && types->length() > 1) {
     Drop(2);
     return HandlePolymorphicStoreNamedField(
-        expr->position(), id, object, value, types, name);
+        expr->position(), ast_id, object, value, types, name);
   } else {
     Drop(2);
     instr = BuildStoreNamedGeneric(object, name, value);
@@ -5042,13 +4984,71 @@ void HOptimizedGraphBuilder::BuildStoreNamed(Expression* expr,
   instr->set_position(expr->position());
   AddInstruction(instr);
   if (instr->HasObservableSideEffects()) {
-    Add<HSimulate>(id, REMOVABLE_SIMULATE);
+    Add<HSimulate>(ast_id, REMOVABLE_SIMULATE);
   }
   if (!ast_context()->IsEffect()) Drop(1);
   return ast_context()->ReturnValue(value);
 }
 
 
+void HOptimizedGraphBuilder::HandlePropertyAssignment(Assignment* expr) {
+  Property* prop = expr->target()->AsProperty();
+  ASSERT(prop != NULL);
+  CHECK_ALIVE(VisitForValue(prop->obj()));
+  if (!prop->key()->IsPropertyName()) {
+    CHECK_ALIVE(VisitForValue(prop->key()));
+  }
+  CHECK_ALIVE(VisitForValue(expr->value()));
+  BuildStore(expr, prop, expr->id(),
+             expr->AssignmentId(), expr->IsUninitialized());
+}
+
+
+// Because not every expression has a position and there is not common
+// superclass of Assignment and CountOperation, we cannot just pass the
+// owning expression instead of position and ast_id separately.
+void HOptimizedGraphBuilder::HandleGlobalVariableAssignment(
+    Variable* var,
+    HValue* value,
+    int position,
+    BailoutId ast_id) {
+  LookupResult lookup(isolate());
+  GlobalPropertyAccess type = LookupGlobalProperty(var, &lookup, true);
+  if (type == kUseCell) {
+    Handle<GlobalObject> global(current_info()->global_object());
+    Handle<PropertyCell> cell(global->GetPropertyCell(&lookup));
+    if (cell->type()->IsConstant()) {
+      IfBuilder builder(this);
+      HValue* constant = Add<HConstant>(cell->type()->AsConstant());
+      if (cell->type()->AsConstant()->IsNumber()) {
+        builder.If<HCompareNumericAndBranch>(value, constant, Token::EQ);
+      } else {
+        builder.If<HCompareObjectEqAndBranch>(value, constant);
+      }
+      builder.Then();
+      builder.Else();
+      Add<HDeoptimize>("Constant global variable assignment",
+                       Deoptimizer::EAGER);
+      builder.End();
+    }
+    HInstruction* instr =
+        Add<HStoreGlobalCell>(value, cell, lookup.GetPropertyDetails());
+    instr->set_position(position);
+    if (instr->HasObservableSideEffects()) {
+      Add<HSimulate>(ast_id, REMOVABLE_SIMULATE);
+    }
+  } else {
+    HGlobalObject* global_object = Add<HGlobalObject>();
+    HStoreGlobalGeneric* instr =
+        Add<HStoreGlobalGeneric>(global_object, var->name(),
+                                 value, function_strict_mode_flag());
+    instr->set_position(position);
+    ASSERT(instr->HasObservableSideEffects());
+    Add<HSimulate>(ast_id, REMOVABLE_SIMULATE);
+  }
+}
+
+
 void HOptimizedGraphBuilder::HandleCompoundAssignment(Assignment* expr) {
   Expression* target = expr->target();
   VariableProxy* proxy = target->AsVariableProxy();
@@ -5130,62 +5130,30 @@ void HOptimizedGraphBuilder::HandleCompoundAssignment(Assignment* expr) {
     return ast_context()->ReturnValue(Pop());
 
   } else if (prop != NULL) {
-    if (prop->key()->IsPropertyName()) {
-      // Named property.
-      CHECK_ALIVE(VisitForValue(prop->obj()));
-      HValue* object = Top();
-      CHECK_ALIVE(PushLoad(prop, object, expr->position()));
-
-      CHECK_ALIVE(VisitForValue(expr->value()));
-      HValue* right = Pop();
-      HValue* left = Pop();
-
-      HInstruction* instr = BuildBinaryOperation(operation, left, right);
-      PushAndAdd(instr);
-      if (instr->HasObservableSideEffects()) {
-        Add<HSimulate>(operation->id(), REMOVABLE_SIMULATE);
-      }
-
-      return BuildStoreNamed(
-          expr, expr->id(), expr->AssignmentId(), prop, object, instr);
-    } else {
-      // Keyed property.
-      CHECK_ALIVE(VisitForValue(prop->obj()));
+    CHECK_ALIVE(VisitForValue(prop->obj()));
+    HValue* object = Top();
+    HValue* key = NULL;
+    if ((!prop->IsStringLength() &&
+         !prop->IsFunctionPrototype() &&
+         !prop->key()->IsPropertyName()) ||
+        prop->IsStringAccess()) {
       CHECK_ALIVE(VisitForValue(prop->key()));
-      HValue* obj = environment()->ExpressionStackAt(1);
-      HValue* key = environment()->ExpressionStackAt(0);
-
-      bool has_side_effects = false;
-      HValue* load = HandleKeyedElementAccess(
-          obj, key, NULL, prop, prop->LoadId(), RelocInfo::kNoPosition,
-          false,  // is_store
-          &has_side_effects);
-      Push(load);
-      if (has_side_effects) Add<HSimulate>(prop->LoadId(), REMOVABLE_SIMULATE);
-
-      CHECK_ALIVE(VisitForValue(expr->value()));
-      HValue* right = Pop();
-      HValue* left = Pop();
-
-      HInstruction* instr = BuildBinaryOperation(operation, left, right);
-      PushAndAdd(instr);
-      if (instr->HasObservableSideEffects()) {
-        Add<HSimulate>(operation->id(), REMOVABLE_SIMULATE);
-      }
+      key = Top();
+    }
 
-      HandleKeyedElementAccess(obj, key, instr, expr, expr->AssignmentId(),
-                               RelocInfo::kNoPosition,
-                               true,  // is_store
-                               &has_side_effects);
+    CHECK_ALIVE(PushLoad(prop, object, key, expr->position()));
 
-      // Drop the simulated receiver, key, and value.  Return the value.
-      Drop(3);
-      Push(instr);
-      ASSERT(has_side_effects);  // Stores always have side effects.
-      Add<HSimulate>(expr->AssignmentId(), REMOVABLE_SIMULATE);
-      return ast_context()->ReturnValue(Pop());
-    }
+    CHECK_ALIVE(VisitForValue(expr->value()));
+    HValue* right = Pop();
+    HValue* left = Pop();
 
+    HInstruction* instr = BuildBinaryOperation(operation, left, right);
+    PushAndAdd(instr);
+    if (instr->HasObservableSideEffects()) {
+      Add<HSimulate>(operation->id(), REMOVABLE_SIMULATE);
+    }
+    BuildStore(expr, prop, expr->id(),
+               expr->AssignmentId(), expr->IsUninitialized());
   } else {
     return Bailout(kInvalidLhsInCompoundAssignment);
   }
@@ -5840,9 +5808,11 @@ bool HOptimizedGraphBuilder::TryArgumentsAccess(Property* expr) {
 
 void HOptimizedGraphBuilder::PushLoad(Property* expr,
                                       HValue* object,
+                                      HValue* key,
                                       int position) {
   ValueContext for_value(this, ARGUMENTS_NOT_ALLOWED);
   Push(object);
+  if (key != NULL) Push(key);
   BuildLoad(expr, position, expr->LoadId());
 }
 
@@ -5858,7 +5828,6 @@ void HOptimizedGraphBuilder::BuildLoad(Property* expr,
         AddInstruction(HCheckInstanceType::NewIsString(string, zone()));
     instr = BuildLoadStringLength(string, checkstring);
   } else if (expr->IsStringAccess()) {
-    CHECK_ALIVE(VisitForValue(expr->key()));
     HValue* index = Pop();
     HValue* string = Pop();
     HValue* context = environment()->context();
@@ -5902,8 +5871,6 @@ void HOptimizedGraphBuilder::BuildLoad(Property* expr,
     }
 
   } else {
-    CHECK_ALIVE(VisitForValue(expr->key()));
-
     HValue* key = Pop();
     HValue* obj = Pop();
 
@@ -5936,6 +5903,13 @@ void HOptimizedGraphBuilder::VisitProperty(Property* expr) {
   if (TryArgumentsAccess(expr)) return;
 
   CHECK_ALIVE(VisitForValue(expr->obj()));
+  if ((!expr->IsStringLength() &&
+       !expr->IsFunctionPrototype() &&
+       !expr->key()->IsPropertyName()) ||
+      expr->IsStringAccess()) {
+    CHECK_ALIVE(VisitForValue(expr->key()));
+  }
+
   BuildLoad(expr, expr->position(), expr->id());
 }
 
@@ -7478,16 +7452,18 @@ HInstruction* HOptimizedGraphBuilder::BuildIncrement(
 }
 
 
-void HOptimizedGraphBuilder::BuildStoreInEffect(Expression* expr,
-                                                Property* prop,
-                                                BailoutId ast_id,
-                                                BailoutId return_id,
-                                                HValue* object,
-                                                HValue* value) {
+void HOptimizedGraphBuilder::BuildStoreForEffect(Expression* expr,
+                                                 Property* prop,
+                                                 BailoutId ast_id,
+                                                 BailoutId return_id,
+                                                 HValue* object,
+                                                 HValue* key,
+                                                 HValue* value) {
   EffectContext for_effect(this);
   Push(object);
+  if (key != NULL) Push(key);
   Push(value);
-  BuildStoreNamed(expr, ast_id, return_id, prop, object, value);
+  BuildStore(expr, prop, ast_id, return_id);
 }
 
 
@@ -7567,69 +7543,42 @@ void HOptimizedGraphBuilder::VisitCountOperation(CountOperation* expr) {
         return Bailout(kLookupVariableInCountOperation);
     }
 
-  } else {
-    // Argument of the count operation is a property.
-    ASSERT(prop != NULL);
+    Drop(returns_original_input ? 2 : 1);
+    return ast_context()->ReturnValue(expr->is_postfix() ? input : after);
+  }
 
-    if (prop->key()->IsPropertyName()) {
-      // Named property.
-      if (returns_original_input) Push(graph()->GetConstantUndefined());
+  // Argument of the count operation is a property.
+  ASSERT(prop != NULL);
+  if (returns_original_input) Push(graph()->GetConstantUndefined());
 
-      CHECK_ALIVE(VisitForValue(prop->obj()));
-      HValue* object = Top();
-      CHECK_ALIVE(PushLoad(prop, object, expr->position()));
+  CHECK_ALIVE(VisitForValue(prop->obj()));
+  HValue* object = Top();
 
-      after = BuildIncrement(returns_original_input, expr);
+  HValue* key = NULL;
+  if ((!prop->IsStringLength() &&
+       !prop->IsFunctionPrototype() &&
+       !prop->key()->IsPropertyName()) ||
+      prop->IsStringAccess()) {
+    CHECK_ALIVE(VisitForValue(prop->key()));
+    key = Top();
+  }
 
-      if (returns_original_input) {
-        HValue* result = Pop();
-        HValue* object = Pop();
-        environment()->SetExpressionStackAt(0, result);
-        CHECK_ALIVE(BuildStoreInEffect(
-            expr, prop, expr->id(), expr->AssignmentId(), object, after));
-        return ast_context()->ReturnValue(Pop());
-      }
+  CHECK_ALIVE(PushLoad(prop, object, key, expr->position()));
 
-      return BuildStoreNamed(
-          expr, expr->id(), expr->AssignmentId(), prop, object, after);
-    } else {
-      // Keyed property.
-      if (returns_original_input) Push(graph()->GetConstantUndefined());
+  after = BuildIncrement(returns_original_input, expr);
 
-      CHECK_ALIVE(VisitForValue(prop->obj()));
-      CHECK_ALIVE(VisitForValue(prop->key()));
-      HValue* obj = environment()->ExpressionStackAt(1);
-      HValue* key = environment()->ExpressionStackAt(0);
-
-      bool has_side_effects = false;
-      HValue* load = HandleKeyedElementAccess(
-          obj, key, NULL, prop, prop->LoadId(), RelocInfo::kNoPosition,
-          false,  // is_store
-          &has_side_effects);
-      Push(load);
-      if (has_side_effects) Add<HSimulate>(prop->LoadId(), REMOVABLE_SIMULATE);
-
-      after = BuildIncrement(returns_original_input, expr);
-      input = environment()->ExpressionStackAt(0);
-
-      HandleKeyedElementAccess(obj, key, after, expr, expr->AssignmentId(),
-                               RelocInfo::kNoPosition,
-                               true,  // is_store
-                               &has_side_effects);
-
-      // Drop the key and the original value from the bailout environment.
-      // Overwrite the receiver with the result of the operation, and the
-      // placeholder with the original value if necessary.
-      Drop(2);
-      environment()->SetExpressionStackAt(0, after);
-      if (returns_original_input) environment()->SetExpressionStackAt(1, input);
-      ASSERT(has_side_effects);  // Stores always have side effects.
-      Add<HSimulate>(expr->AssignmentId(), REMOVABLE_SIMULATE);
-    }
+  if (returns_original_input) {
+    input = Pop();
+    // Drop object and key to push it again in the effect context below.
+    Drop(key == NULL ? 1 : 2);
+    environment()->SetExpressionStackAt(0, input);
+    CHECK_ALIVE(BuildStoreForEffect(
+        expr, prop, expr->id(), expr->AssignmentId(), object, key, after));
+    return ast_context()->ReturnValue(Pop());
   }
 
-  Drop(returns_original_input ? 2 : 1);
-  return ast_context()->ReturnValue(expr->is_postfix() ? input : after);
+  environment()->SetExpressionStackAt(0, after);
+  return BuildStore(expr, prop, expr->id(), expr->AssignmentId());
 }