From e25e6ab25d9336219b6e8caf5c3329cf443ce07a Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Fri, 6 Sep 2013 13:06:39 +0000 Subject: [PATCH] Let BuildStore/BuildLoad distinguish between keyed/named load/stores. R=bmeurer@chromium.org Review URL: https://chromiumcodereview.appspot.com/23537024 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16575 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/hydrogen.cc | 361 ++++++++++++++++++++++++-------------------------------- src/hydrogen.h | 27 +++-- 2 files changed, 169 insertions(+), 219 deletions(-) diff --git a/src/hydrogen.cc b/src/hydrogen.cc index bd9b0d6..3cbe1cb 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("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(expr->AssignmentId(), REMOVABLE_SIMULATE); + Add(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 global(current_info()->global_object()); - Handle cell(global->GetPropertyCell(&lookup)); - if (cell->type()->IsConstant()) { - IfBuilder builder(this); - HValue* constant = Add(cell->type()->AsConstant()); - if (cell->type()->AsConstant()->IsNumber()) { - builder.If(value, constant, Token::EQ); - } else { - builder.If(value, constant); - } - builder.Then(); - builder.Else(); - Add("Constant global variable assignment", - Deoptimizer::EAGER); - builder.End(); - } - HInstruction* instr = - Add(value, cell, lookup.GetPropertyDetails()); - instr->set_position(position); - if (instr->HasObservableSideEffects()) { - Add(ast_id, REMOVABLE_SIMULATE); - } - } else { - HGlobalObject* global_object = Add(); - HStoreGlobalGeneric* instr = - Add(global_object, var->name(), - value, function_strict_mode_flag()); - instr->set_position(position); - ASSERT(instr->HasObservableSideEffects()); - Add(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("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 name = Handle::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(id, REMOVABLE_SIMULATE); + Add(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 global(current_info()->global_object()); + Handle cell(global->GetPropertyCell(&lookup)); + if (cell->type()->IsConstant()) { + IfBuilder builder(this); + HValue* constant = Add(cell->type()->AsConstant()); + if (cell->type()->AsConstant()->IsNumber()) { + builder.If(value, constant, Token::EQ); + } else { + builder.If(value, constant); + } + builder.Then(); + builder.Else(); + Add("Constant global variable assignment", + Deoptimizer::EAGER); + builder.End(); + } + HInstruction* instr = + Add(value, cell, lookup.GetPropertyDetails()); + instr->set_position(position); + if (instr->HasObservableSideEffects()) { + Add(ast_id, REMOVABLE_SIMULATE); + } + } else { + HGlobalObject* global_object = Add(); + HStoreGlobalGeneric* instr = + Add(global_object, var->name(), + value, function_strict_mode_flag()); + instr->set_position(position); + ASSERT(instr->HasObservableSideEffects()); + Add(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(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(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(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(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(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(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(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()); } diff --git a/src/hydrogen.h b/src/hydrogen.h index a12773f..395d1cd 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -2038,21 +2038,22 @@ class HOptimizedGraphBuilder V8_FINAL BailoutId ast_id); void PushLoad(Property* property, HValue* object, + HValue* key, int position); - void BuildStoreInEffect(Expression* expression, - Property* prop, - BailoutId ast_id, - BailoutId return_id, - HValue* object, - HValue* value); - - void BuildStoreNamed(Expression* expression, - BailoutId id, - BailoutId assignment_id, - Property* prop, - HValue* object, - HValue* value); + void BuildStoreForEffect(Expression* expression, + Property* prop, + BailoutId ast_id, + BailoutId return_id, + HValue* object, + HValue* key, + HValue* value); + + void BuildStore(Expression* expression, + Property* prop, + BailoutId ast_id, + BailoutId return_id, + bool is_uninitialized = false); HInstruction* BuildStoreNamedField(HValue* object, Handle name, -- 2.7.4