From 29e15dad16984ba62d63c72149525c61a4e5328e Mon Sep 17 00:00:00 2001 From: titzer Date: Wed, 13 May 2015 05:29:38 -0700 Subject: [PATCH] [turbofan] Add FrameStates before all property accesses. R=jarin@chromium.org BUG= Review URL: https://codereview.chromium.org/1133303006 Cr-Commit-Position: refs/heads/master@{#28392} --- src/compiler/ast-graph-builder.cc | 144 +++++++++++++-------- src/compiler/ast-graph-builder.h | 15 ++- src/compiler/js-generic-lowering.cc | 136 ++++++++++++------- src/compiler/js-type-feedback.cc | 3 - src/compiler/operator-properties.cc | 11 +- .../compiler/js-type-feedback-unittest.cc | 9 +- .../compiler/js-typed-lowering-unittest.cc | 12 +- 7 files changed, 205 insertions(+), 125 deletions(-) diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 629ea18..2e6dad8 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -632,6 +632,14 @@ static LhsKind DetermineLhsKind(Expression* expr) { } +// Gets the bailout id just before reading a variable proxy, but only for +// unallocated variables. +static BailoutId BeforeId(VariableProxy* proxy) { + return proxy->var()->location() == Variable::UNALLOCATED ? proxy->BeforeId() + : BailoutId::None(); +} + + static const char* GetDebugParameterName(Zone* zone, Scope* scope, int index) { #if DEBUG const AstRawString* name = scope->parameter(index)->raw_name(); @@ -1643,7 +1651,9 @@ void AstGraphBuilder::VisitClassLiteralContents(ClassLiteral* expr) { if (expr->scope() != NULL) { DCHECK_NOT_NULL(expr->class_variable_proxy()); Variable* var = expr->class_variable_proxy()->var(); - BuildVariableAssignment(var, literal, Token::INIT_CONST, BailoutId::None()); + FrameStateBeforeAndAfter states(this, BailoutId::None()); + BuildVariableAssignment(states, var, literal, Token::INIT_CONST, + BailoutId::None()); } ast_context()->ProduceValue(literal); @@ -1671,7 +1681,9 @@ void AstGraphBuilder::VisitConditional(Conditional* expr) { void AstGraphBuilder::VisitVariableProxy(VariableProxy* expr) { VectorSlotPair pair = CreateVectorSlotPair(expr->VariableFeedbackSlot()); - Node* value = BuildVariableLoad(expr->var(), expr->id(), pair); + FrameStateBeforeAndAfter states(this, BeforeId(expr)); + Node* value = BuildVariableLoad(states, expr->var(), expr->id(), pair, + ast_context()->GetStateCombine()); ast_context()->ProduceValue(value); } @@ -1744,11 +1756,13 @@ void AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) { if (key->value()->IsInternalizedString()) { if (property->emit_store()) { VisitForValue(property->value()); + FrameStateBeforeAndAfter states(this, property->value()->id()); Node* value = environment()->Pop(); Handle name = key->AsPropertyName(); Node* store = BuildNamedStore(literal, name, value, TypeFeedbackId::None()); - PrepareFrameState(store, key->id()); + states.AddToNode(store, key->id(), + OutputFrameStateCombine::Ignore()); BuildSetHomeObject(value, literal, property->value()); } else { VisitForEffect(property->value()); @@ -1945,34 +1959,32 @@ void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value, switch (assign_type) { case VARIABLE: { Variable* var = expr->AsVariableProxy()->var(); - BuildVariableAssignment(var, value, Token::ASSIGN, bailout_id); + FrameStateBeforeAndAfter states(this, BailoutId::None()); + BuildVariableAssignment(states, var, value, Token::ASSIGN, bailout_id); break; } case NAMED_PROPERTY: { environment()->Push(value); VisitForValue(property->obj()); + FrameStateBeforeAndAfter states(this, property->obj()->id()); Node* object = environment()->Pop(); value = environment()->Pop(); Handle name = property->key()->AsLiteral()->AsPropertyName(); Node* store = BuildNamedStore(object, name, value, TypeFeedbackId::None()); - PrepareFrameState(store, bailout_id); + states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore()); break; } case KEYED_PROPERTY: { environment()->Push(value); VisitForValue(property->obj()); VisitForValue(property->key()); - { - // TODO(jarin) Provide a real frame state before. - FrameStateBeforeAndAfter states(this, BailoutId::None()); - Node* key = environment()->Pop(); - Node* object = environment()->Pop(); - value = environment()->Pop(); - Node* store = - BuildKeyedStore(object, key, value, TypeFeedbackId::None()); - states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore()); - } + FrameStateBeforeAndAfter states(this, property->key()->id()); + Node* key = environment()->Pop(); + Node* object = environment()->Pop(); + value = environment()->Pop(); + Node* store = BuildKeyedStore(object, key, value, TypeFeedbackId::None()); + states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore()); break; } } @@ -2018,7 +2030,10 @@ void AstGraphBuilder::VisitAssignment(Assignment* expr) { VariableProxy* proxy = expr->target()->AsVariableProxy(); VectorSlotPair pair = CreateVectorSlotPair(proxy->VariableFeedbackSlot()); - old_value = BuildVariableLoad(proxy->var(), expr->target()->id(), pair); + FrameStateBeforeAndAfter states(this, BeforeId(proxy)); + old_value = + BuildVariableLoad(states, proxy->var(), expr->target()->id(), pair, + OutputFrameStateCombine::Push()); break; } case NAMED_PROPERTY: { @@ -2026,10 +2041,11 @@ void AstGraphBuilder::VisitAssignment(Assignment* expr) { Handle name = property->key()->AsLiteral()->AsPropertyName(); VectorSlotPair pair = CreateVectorSlotPair(property->PropertyFeedbackSlot()); + FrameStateBeforeAndAfter states(this, property->obj()->id()); old_value = BuildNamedLoad(object, name, pair, property->PropertyFeedbackId()); - PrepareFrameState(old_value, property->LoadId(), - OutputFrameStateCombine::Push()); + states.AddToNode(old_value, property->LoadId(), + OutputFrameStateCombine::Push()); break; } case KEYED_PROPERTY: { @@ -2037,10 +2053,11 @@ void AstGraphBuilder::VisitAssignment(Assignment* expr) { Node* object = environment()->Peek(1); VectorSlotPair pair = CreateVectorSlotPair(property->PropertyFeedbackSlot()); + FrameStateBeforeAndAfter states(this, property->key()->id()); old_value = BuildKeyedLoad(object, key, pair, property->PropertyFeedbackId()); - PrepareFrameState(old_value, property->LoadId(), - OutputFrameStateCombine::Push()); + states.AddToNode(old_value, property->LoadId(), + OutputFrameStateCombine::Push()); break; } } @@ -2072,8 +2089,8 @@ void AstGraphBuilder::VisitAssignment(Assignment* expr) { switch (assign_type) { case VARIABLE: { Variable* variable = expr->target()->AsVariableProxy()->var(); - BuildVariableAssignment(variable, value, expr->op(), expr->id(), - ast_context()->GetStateCombine()); + BuildVariableAssignment(store_states, variable, value, expr->op(), + expr->id(), ast_context()->GetStateCombine()); break; } case NAMED_PROPERTY: { @@ -2120,17 +2137,20 @@ void AstGraphBuilder::VisitProperty(Property* expr) { VectorSlotPair pair = CreateVectorSlotPair(expr->PropertyFeedbackSlot()); if (expr->key()->IsPropertyName()) { VisitForValue(expr->obj()); + FrameStateBeforeAndAfter states(this, expr->obj()->id()); Node* object = environment()->Pop(); Handle name = expr->key()->AsLiteral()->AsPropertyName(); value = BuildNamedLoad(object, name, pair, expr->PropertyFeedbackId()); + states.AddToNode(value, expr->id(), ast_context()->GetStateCombine()); } else { VisitForValue(expr->obj()); VisitForValue(expr->key()); + FrameStateBeforeAndAfter states(this, expr->key()->id()); Node* key = environment()->Pop(); Node* object = environment()->Pop(); value = BuildKeyedLoad(object, key, pair, expr->PropertyFeedbackId()); + states.AddToNode(value, expr->id(), ast_context()->GetStateCombine()); } - PrepareFrameState(value, expr->id(), ast_context()->GetStateCombine()); ast_context()->ProduceValue(value); } @@ -2149,8 +2169,10 @@ void AstGraphBuilder::VisitCall(Call* expr) { case Call::GLOBAL_CALL: { VariableProxy* proxy = callee->AsVariableProxy(); VectorSlotPair pair = CreateVectorSlotPair(proxy->VariableFeedbackSlot()); + FrameStateBeforeAndAfter states(this, BeforeId(proxy)); callee_value = - BuildVariableLoad(proxy->var(), expr->expression()->id(), pair); + BuildVariableLoad(states, proxy->var(), expr->expression()->id(), + pair, OutputFrameStateCombine::Push()); receiver_value = jsgraph()->UndefinedConstant(); break; } @@ -2175,17 +2197,21 @@ void AstGraphBuilder::VisitCall(Call* expr) { VectorSlotPair pair = CreateVectorSlotPair(property->PropertyFeedbackSlot()); if (property->key()->IsPropertyName()) { + FrameStateBeforeAndAfter states(this, property->obj()->id()); Handle name = property->key()->AsLiteral()->AsPropertyName(); callee_value = BuildNamedLoad(object, name, pair, property->PropertyFeedbackId()); + states.AddToNode(callee_value, property->LoadId(), + OutputFrameStateCombine::Push()); } else { VisitForValue(property->key()); + FrameStateBeforeAndAfter states(this, property->key()->id()); Node* key = environment()->Pop(); callee_value = BuildKeyedLoad(object, key, pair, property->PropertyFeedbackId()); + states.AddToNode(callee_value, property->LoadId(), + OutputFrameStateCombine::Push()); } - PrepareFrameState(callee_value, property->LoadId(), - OutputFrameStateCombine::Push()); receiver_value = environment()->Pop(); // Note that a PROPERTY_CALL requires the receiver to be wrapped into an // object for sloppy callees. This could also be modeled explicitly here, @@ -2295,12 +2321,12 @@ void AstGraphBuilder::VisitCallJSRuntime(CallRuntime* expr) { CallFunctionFlags flags = NO_CALL_FUNCTION_FLAGS; Node* receiver_value = BuildLoadBuiltinsObject(); VectorSlotPair pair = CreateVectorSlotPair(expr->CallRuntimeFeedbackSlot()); + // TODO(jarin): bailout ids for runtime calls. + FrameStateBeforeAndAfter states(this, BailoutId::None()); Node* callee_value = BuildNamedLoad(receiver_value, name, pair, expr->CallRuntimeFeedbackId()); - // TODO(jarin): Find/create a bailout id to deoptimize to (crankshaft - // refuses to optimize functions with jsruntime calls). - PrepareFrameState(callee_value, BailoutId::None(), - OutputFrameStateCombine::Push()); + states.AddToNode(callee_value, BailoutId::None(), + OutputFrameStateCombine::Push()); environment()->Push(callee_value); environment()->Push(receiver_value); @@ -2374,35 +2400,39 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) { case VARIABLE: { VariableProxy* proxy = expr->expression()->AsVariableProxy(); VectorSlotPair pair = CreateVectorSlotPair(proxy->VariableFeedbackSlot()); + FrameStateBeforeAndAfter states(this, BeforeId(proxy)); old_value = - BuildVariableLoad(proxy->var(), expr->expression()->id(), pair); + BuildVariableLoad(states, proxy->var(), expr->expression()->id(), + pair, OutputFrameStateCombine::Push()); stack_depth = 0; break; } case NAMED_PROPERTY: { VisitForValue(property->obj()); + FrameStateBeforeAndAfter states(this, property->obj()->id()); Node* object = environment()->Top(); Handle name = property->key()->AsLiteral()->AsPropertyName(); VectorSlotPair pair = CreateVectorSlotPair(property->PropertyFeedbackSlot()); old_value = BuildNamedLoad(object, name, pair, property->PropertyFeedbackId()); - PrepareFrameState(old_value, property->LoadId(), - OutputFrameStateCombine::Push()); + states.AddToNode(old_value, property->LoadId(), + OutputFrameStateCombine::Push()); stack_depth = 1; break; } case KEYED_PROPERTY: { VisitForValue(property->obj()); VisitForValue(property->key()); + FrameStateBeforeAndAfter states(this, property->key()->id()); Node* key = environment()->Top(); Node* object = environment()->Peek(1); VectorSlotPair pair = CreateVectorSlotPair(property->PropertyFeedbackSlot()); old_value = BuildKeyedLoad(object, key, pair, property->PropertyFeedbackId()); - PrepareFrameState(old_value, property->LoadId(), - OutputFrameStateCombine::Push()); + states.AddToNode(old_value, property->LoadId(), + OutputFrameStateCombine::Push()); stack_depth = 2; break; } @@ -2438,7 +2468,7 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) { case VARIABLE: { Variable* variable = expr->expression()->AsVariableProxy()->var(); environment()->Push(value); - BuildVariableAssignment(variable, value, expr->op(), + BuildVariableAssignment(store_states, variable, value, expr->op(), expr->AssignmentId()); environment()->Pop(); break; @@ -2449,7 +2479,8 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) { Node* store = BuildNamedStore(object, name, value, expr->CountStoreFeedbackId()); environment()->Push(value); - PrepareFrameState(store, expr->AssignmentId()); + store_states.AddToNode(store, expr->AssignmentId(), + OutputFrameStateCombine::Ignore()); environment()->Pop(); break; } @@ -2639,8 +2670,10 @@ void AstGraphBuilder::VisitTypeof(UnaryOperation* expr) { // perform a non-contextual load in case the operand is a variable proxy. VariableProxy* proxy = expr->expression()->AsVariableProxy(); VectorSlotPair pair = CreateVectorSlotPair(proxy->VariableFeedbackSlot()); - operand = BuildVariableLoad(proxy->var(), expr->expression()->id(), pair, - NOT_CONTEXTUAL); + FrameStateBeforeAndAfter states(this, BeforeId(proxy)); + operand = + BuildVariableLoad(states, proxy->var(), expr->expression()->id(), pair, + OutputFrameStateCombine::Push(), NOT_CONTEXTUAL); } else { VisitForValue(expr->expression()); operand = environment()->Pop(); @@ -2810,7 +2843,9 @@ Node* AstGraphBuilder::BuildArgumentsObject(Variable* arguments) { // Assign the object to the arguments variable. DCHECK(arguments->IsContextSlot() || arguments->IsStackAllocated()); // This should never lazy deopt, so it is fine to send invalid bailout id. - BuildVariableAssignment(arguments, object, Token::ASSIGN, BailoutId::None()); + FrameStateBeforeAndAfter states(this, BailoutId::None()); + BuildVariableAssignment(states, arguments, object, Token::ASSIGN, + BailoutId::None()); return object; } @@ -2826,7 +2861,9 @@ Node* AstGraphBuilder::BuildRestArgumentsArray(Variable* rest, int index) { // Assign the object to the rest array DCHECK(rest->IsContextSlot() || rest->IsStackAllocated()); // This should never lazy deopt, so it is fine to send invalid bailout id. - BuildVariableAssignment(rest, object, Token::ASSIGN, BailoutId::None()); + FrameStateBeforeAndAfter states(this, BailoutId::None()); + BuildVariableAssignment(states, rest, object, Token::ASSIGN, + BailoutId::None()); return object; } @@ -2875,9 +2912,11 @@ Node* AstGraphBuilder::BuildThrowIfStaticPrototype(Node* name, } -Node* AstGraphBuilder::BuildVariableLoad(Variable* variable, +Node* AstGraphBuilder::BuildVariableLoad(FrameStateBeforeAndAfter& states, + Variable* variable, BailoutId bailout_id, const VectorSlotPair& feedback, + OutputFrameStateCombine combine, ContextualMode contextual_mode) { Node* the_hole = jsgraph()->TheHoleConstant(); VariableMode mode = variable->mode(); @@ -2888,7 +2927,7 @@ Node* AstGraphBuilder::BuildVariableLoad(Variable* variable, Handle name = variable->name(); Node* node = BuildNamedLoad(global, name, feedback, TypeFeedbackId::None(), contextual_mode); - PrepareFrameState(node, bailout_id, OutputFrameStateCombine::Push()); + states.AddToNode(node, bailout_id, combine); return node; } case Variable::PARAMETER: @@ -2951,9 +2990,9 @@ Node* AstGraphBuilder::BuildVariableLoad(Variable* variable, } -Node* AstGraphBuilder::BuildVariableDelete( - Variable* variable, BailoutId bailout_id, - OutputFrameStateCombine state_combine) { +Node* AstGraphBuilder::BuildVariableDelete(Variable* variable, + BailoutId bailout_id, + OutputFrameStateCombine combine) { switch (variable->location()) { case Variable::UNALLOCATED: { // Global var, const, or let variable. @@ -2961,7 +3000,7 @@ Node* AstGraphBuilder::BuildVariableDelete( Node* name = jsgraph()->Constant(variable->name()); const Operator* op = javascript()->DeleteProperty(language_mode()); Node* result = NewNode(op, global, name); - PrepareFrameState(result, bailout_id, state_combine); + PrepareFrameState(result, bailout_id, combine); return result; } case Variable::PARAMETER: @@ -2975,7 +3014,7 @@ Node* AstGraphBuilder::BuildVariableDelete( const Operator* op = javascript()->CallRuntime(Runtime::kDeleteLookupSlot, 2); Node* result = NewNode(op, current_context(), name); - PrepareFrameState(result, bailout_id, state_combine); + PrepareFrameState(result, bailout_id, combine); return result; } } @@ -2985,8 +3024,8 @@ Node* AstGraphBuilder::BuildVariableDelete( Node* AstGraphBuilder::BuildVariableAssignment( - Variable* variable, Node* value, Token::Value op, BailoutId bailout_id, - OutputFrameStateCombine combine) { + FrameStateBeforeAndAfter& states, Variable* variable, Node* value, + Token::Value op, BailoutId bailout_id, OutputFrameStateCombine combine) { Node* the_hole = jsgraph()->TheHoleConstant(); VariableMode mode = variable->mode(); switch (variable->location()) { @@ -2996,7 +3035,7 @@ Node* AstGraphBuilder::BuildVariableAssignment( Handle name = variable->name(); Node* store = BuildNamedStore(global, name, value, TypeFeedbackId::None()); - PrepareFrameState(store, bailout_id, combine); + states.AddToNode(store, bailout_id, combine); return store; } case Variable::PARAMETER: @@ -3215,9 +3254,10 @@ Node* AstGraphBuilder::BuildSetHomeObject(Node* value, Node* home_object, Expression* expr) { if (!FunctionLiteral::NeedsHomeObject(expr)) return value; Handle name = isolate()->factory()->home_object_symbol(); + FrameStateBeforeAndAfter states(this, BailoutId::None()); Node* store = BuildNamedStore(value, name, home_object, TypeFeedbackId::None()); - PrepareFrameState(store, BailoutId::None()); + states.AddToNode(store, BailoutId::None(), OutputFrameStateCombine::Ignore()); return store; } diff --git a/src/compiler/ast-graph-builder.h b/src/compiler/ast-graph-builder.h index 83a488f..ce959b2 100644 --- a/src/compiler/ast-graph-builder.h +++ b/src/compiler/ast-graph-builder.h @@ -255,14 +255,15 @@ class AstGraphBuilder : public AstVisitor { Node* BuildRestArgumentsArray(Variable* rest, int index); // Builders for variable load and assignment. - Node* BuildVariableAssignment(Variable* var, Node* value, Token::Value op, - BailoutId bailout_id, - OutputFrameStateCombine state_combine = - OutputFrameStateCombine::Ignore()); + Node* BuildVariableAssignment( + FrameStateBeforeAndAfter& states, Variable* var, Node* value, + Token::Value op, BailoutId bailout_id, + OutputFrameStateCombine combine = OutputFrameStateCombine::Ignore()); Node* BuildVariableDelete(Variable* var, BailoutId bailout_id, - OutputFrameStateCombine state_combine); - Node* BuildVariableLoad(Variable* var, BailoutId bailout_id, - const VectorSlotPair& feedback, + OutputFrameStateCombine combine); + Node* BuildVariableLoad(FrameStateBeforeAndAfter& states, Variable* var, + BailoutId bailout_id, const VectorSlotPair& feedback, + OutputFrameStateCombine combine, ContextualMode mode = CONTEXTUAL); // Builders for property loads and stores. diff --git a/src/compiler/js-generic-lowering.cc b/src/compiler/js-generic-lowering.cc index c2d5801..638fbd7 100644 --- a/src/compiler/js-generic-lowering.cc +++ b/src/compiler/js-generic-lowering.cc @@ -17,6 +17,19 @@ namespace v8 { namespace internal { namespace compiler { +static CallDescriptor::Flags AdjustFrameStatesForCall(Node* node) { + int count = OperatorProperties::GetFrameStateInputCount(node->op()); + if (count > 1) { + int index = NodeProperties::FirstFrameStateIndex(node) + 1; + do { + node->RemoveInput(index); + } while (--count > 1); + } + return count > 0 ? CallDescriptor::kNeedsFrameState + : CallDescriptor::kNoFlags; +} + + JSGenericLowering::JSGenericLowering(bool is_typing_enabled, JSGraph* jsgraph) : is_typing_enabled_(is_typing_enabled), jsgraph_(jsgraph) {} @@ -52,11 +65,13 @@ Reduction JSGenericLowering::Reduce(Node* node) { } -#define REPLACE_BINARY_OP_IC_CALL(op, token) \ - void JSGenericLowering::Lower##op(Node* node) { \ - ReplaceWithStubCall(node, CodeFactory::BinaryOpIC(isolate(), token, \ - OpParameter(node)), \ - CallDescriptor::kPatchableCallSiteWithNop); \ +#define REPLACE_BINARY_OP_IC_CALL(op, token) \ + void JSGenericLowering::Lower##op(Node* node) { \ + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); \ + ReplaceWithStubCall( \ + node, CodeFactory::BinaryOpIC(isolate(), token, \ + OpParameter(node)), \ + CallDescriptor::kPatchableCallSiteWithNop | flags); \ } REPLACE_BINARY_OP_IC_CALL(JSBitwiseOr, Token::BIT_OR) REPLACE_BINARY_OP_IC_CALL(JSBitwiseXor, Token::BIT_XOR) @@ -118,10 +133,6 @@ static CallDescriptor::Flags FlagsForNode(Node* node) { void JSGenericLowering::ReplaceWithCompareIC(Node* node, Token::Value token) { Callable callable = CodeFactory::CompareIC(isolate(), token, OpParameter(node)); - CallDescriptor* desc_compare = Linkage::GetStubCallDescriptor( - isolate(), zone(), callable.descriptor(), 0, - CallDescriptor::kPatchableCallSiteWithNop | FlagsForNode(node), - Operator::kNoProperties, kMachIntPtr); // Create a new call node asking a CompareIC for help. NodeVector inputs(zone()); @@ -141,6 +152,10 @@ void JSGenericLowering::ReplaceWithCompareIC(Node* node, Token::Value token) { inputs.push_back(NodeProperties::GetEffectInput(node)); inputs.push_back(NodeProperties::GetControlInput(node)); } + CallDescriptor* desc_compare = Linkage::GetStubCallDescriptor( + isolate(), zone(), callable.descriptor(), 0, + CallDescriptor::kPatchableCallSiteWithNop | FlagsForNode(node), + Operator::kNoProperties, kMachIntPtr); Node* compare = graph()->NewNode(common()->Call(desc_compare), static_cast(inputs.size()), &inputs.front()); @@ -194,53 +209,64 @@ void JSGenericLowering::ReplaceWithCompareIC(Node* node, Token::Value token) { void JSGenericLowering::ReplaceWithStubCall(Node* node, Callable callable, CallDescriptor::Flags flags) { - Operator::Properties properties = node->op()->properties(); - flags |= FlagsForNode(node); + const Operator* old_op = node->op(); + Operator::Properties properties = old_op->properties(); CallDescriptor* desc = Linkage::GetStubCallDescriptor( isolate(), zone(), callable.descriptor(), 0, flags, properties); const Operator* new_op = common()->Call(desc); - // Take care of frame states. - int old_frame_state_count = - OperatorProperties::GetFrameStateInputCount(node->op()); - int new_frame_state_count = - (flags & CallDescriptor::kNeedsFrameState) ? 1 : 0; - DCHECK_GE(old_frame_state_count, new_frame_state_count); - // If there are extra frame states, get rid of them. - for (int i = new_frame_state_count; i < old_frame_state_count; i++) { - node->RemoveInput(NodeProperties::FirstFrameStateIndex(node) + - new_frame_state_count); - } - Node* stub_code = jsgraph()->HeapConstant(callable.code()); node->InsertInput(zone(), 0, stub_code); node->set_op(new_op); + +#if 0 && DEBUG + // Check for at most one framestate and that it's at the right position. + int where = -1; + for (int index = 0; index < node->InputCount(); index++) { + if (node->InputAt(index)->opcode() == IrOpcode::kFrameState) { + if (where >= 0) { + V8_Fatal(__FILE__, __LINE__, + "node #%d:%s already has a framestate at index %d", + node->id(), node->op()->mnemonic(), where); + } + where = index; + DCHECK_EQ(NodeProperties::FirstFrameStateIndex(node), where); + DCHECK(flags & CallDescriptor::kNeedsFrameState); + } + } + if (flags & CallDescriptor::kNeedsFrameState) { + DCHECK_GE(where, 0); // should have found a frame state. + } +#endif } void JSGenericLowering::ReplaceWithBuiltinCall(Node* node, Builtins::JavaScript id, int nargs) { + Node* context_input = NodeProperties::GetContextInput(node); + Node* effect_input = NodeProperties::GetEffectInput(node); + + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); Operator::Properties properties = node->op()->properties(); Callable callable = CodeFactory::CallFunction(isolate(), nargs - 1, NO_CALL_FUNCTION_FLAGS); - CallDescriptor* desc = - Linkage::GetStubCallDescriptor(isolate(), zone(), callable.descriptor(), - nargs, FlagsForNode(node), properties); - Node* global_object = graph()->NewNode( - machine()->Load(kMachAnyTagged), NodeProperties::GetContextInput(node), - jsgraph()->IntPtrConstant( - Context::SlotOffset(Context::GLOBAL_OBJECT_INDEX)), - NodeProperties::GetEffectInput(node), graph()->start()); + CallDescriptor* desc = Linkage::GetStubCallDescriptor( + isolate(), zone(), callable.descriptor(), nargs, flags, properties); + Node* global_object = + graph()->NewNode(machine()->Load(kMachAnyTagged), context_input, + jsgraph()->IntPtrConstant( + Context::SlotOffset(Context::GLOBAL_OBJECT_INDEX)), + effect_input, graph()->start()); Node* builtins_object = graph()->NewNode( machine()->Load(kMachAnyTagged), global_object, jsgraph()->IntPtrConstant(GlobalObject::kBuiltinsOffset - kHeapObjectTag), - NodeProperties::GetEffectInput(node), graph()->start()); + effect_input, graph()->start()); Node* function = graph()->NewNode( machine()->Load(kMachAnyTagged), builtins_object, jsgraph()->IntPtrConstant(JSBuiltinsObject::OffsetOfFunctionWithId(id) - kHeapObjectTag), - NodeProperties::GetEffectInput(node), graph()->start()); + effect_input, graph()->start()); Node* stub_code = jsgraph()->HeapConstant(callable.code()); node->InsertInput(zone(), 0, stub_code); node->InsertInput(zone(), 1, function); @@ -268,26 +294,32 @@ void JSGenericLowering::ReplaceWithRuntimeCall(Node* node, void JSGenericLowering::LowerJSUnaryNot(Node* node) { Callable callable = CodeFactory::ToBoolean( isolate(), ToBooleanStub::RESULT_AS_INVERSE_ODDBALL); - ReplaceWithStubCall(node, callable, CallDescriptor::kPatchableCallSite); + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); + ReplaceWithStubCall(node, callable, + CallDescriptor::kPatchableCallSite | flags); } void JSGenericLowering::LowerJSTypeOf(Node* node) { Callable callable = CodeFactory::Typeof(isolate()); - ReplaceWithStubCall(node, callable, CallDescriptor::kNoFlags); + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); + ReplaceWithStubCall(node, callable, flags); } void JSGenericLowering::LowerJSToBoolean(Node* node) { Callable callable = CodeFactory::ToBoolean(isolate(), ToBooleanStub::RESULT_AS_ODDBALL); - ReplaceWithStubCall(node, callable, CallDescriptor::kPatchableCallSite); + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); + ReplaceWithStubCall(node, callable, + CallDescriptor::kPatchableCallSite | flags); } void JSGenericLowering::LowerJSToNumber(Node* node) { Callable callable = CodeFactory::ToNumber(isolate()); - ReplaceWithStubCall(node, callable, FlagsForNode(node)); + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); + ReplaceWithStubCall(node, callable, flags); } @@ -307,6 +339,7 @@ void JSGenericLowering::LowerJSToObject(Node* node) { void JSGenericLowering::LowerJSLoadProperty(Node* node) { + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); const LoadPropertyParameters& p = LoadPropertyParametersOf(node->op()); Callable callable = CodeFactory::KeyedLoadICInOptimizedCode(isolate(), UNINITIALIZED); @@ -315,11 +348,13 @@ void JSGenericLowering::LowerJSLoadProperty(Node* node) { node->InsertInput(zone(), 3, jsgraph()->HeapConstant(p.feedback().vector())); } - ReplaceWithStubCall(node, callable, CallDescriptor::kPatchableCallSite); + ReplaceWithStubCall(node, callable, + CallDescriptor::kPatchableCallSite | flags); } void JSGenericLowering::LowerJSLoadNamed(Node* node) { + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); const LoadNamedParameters& p = LoadNamedParametersOf(node->op()); Callable callable = p.load_ic() == NAMED @@ -332,26 +367,31 @@ void JSGenericLowering::LowerJSLoadNamed(Node* node) { node->InsertInput(zone(), 3, jsgraph()->HeapConstant(p.feedback().vector())); } - ReplaceWithStubCall(node, callable, CallDescriptor::kPatchableCallSite); + ReplaceWithStubCall(node, callable, + CallDescriptor::kPatchableCallSite | flags); } void JSGenericLowering::LowerJSStoreProperty(Node* node) { + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); LanguageMode language_mode = OpParameter(node); Callable callable = CodeFactory::KeyedStoreICInOptimizedCode( isolate(), language_mode, UNINITIALIZED); - ReplaceWithStubCall(node, callable, CallDescriptor::kPatchableCallSite); + ReplaceWithStubCall(node, callable, + CallDescriptor::kPatchableCallSite | flags); } void JSGenericLowering::LowerJSStoreNamed(Node* node) { + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); const StoreNamedParameters& p = StoreNamedParametersOf(node->op()); Callable callable = p.store_ic() == NAMED ? CodeFactory::StoreIC(isolate(), p.language_mode()) : CodeFactory::KeyedStoreICInOptimizedCode( isolate(), p.language_mode(), UNINITIALIZED); node->InsertInput(zone(), 1, jsgraph()->HeapConstant(p.name())); - ReplaceWithStubCall(node, callable, CallDescriptor::kPatchableCallSite); + ReplaceWithStubCall(node, callable, + CallDescriptor::kPatchableCallSite | flags); } @@ -373,8 +413,9 @@ void JSGenericLowering::LowerJSInstanceOf(Node* node) { InstanceofStub::kArgsInRegisters); InstanceofStub stub(isolate(), flags); CallInterfaceDescriptor d = stub.GetCallInterfaceDescriptor(); - CallDescriptor* desc = Linkage::GetStubCallDescriptor(isolate(), zone(), d, 0, - FlagsForNode(node)); + CallDescriptor::Flags desc_flags = AdjustFrameStatesForCall(node); + CallDescriptor* desc = + Linkage::GetStubCallDescriptor(isolate(), zone(), d, 0, desc_flags); Node* stub_code = jsgraph()->HeapConstant(stub.GetCode()); node->InsertInput(zone(), 0, stub_code); node->set_op(common()->Call(desc)); @@ -451,8 +492,9 @@ void JSGenericLowering::LowerJSCallConstruct(Node* node) { int arity = OpParameter(node); CallConstructStub stub(isolate(), NO_CALL_CONSTRUCTOR_FLAGS); CallInterfaceDescriptor d = stub.GetCallInterfaceDescriptor(); - CallDescriptor* desc = Linkage::GetStubCallDescriptor( - isolate(), zone(), d, arity, FlagsForNode(node)); + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); + CallDescriptor* desc = + Linkage::GetStubCallDescriptor(isolate(), zone(), d, arity, flags); Node* stub_code = jsgraph()->HeapConstant(stub.GetCode()); Node* construct = NodeProperties::GetValueInput(node, 0); node->InsertInput(zone(), 0, stub_code); @@ -513,9 +555,9 @@ void JSGenericLowering::LowerJSCallFunction(Node* node) { int arg_count = static_cast(p.arity() - 2); CallFunctionStub stub(isolate(), arg_count, p.flags()); CallInterfaceDescriptor d = stub.GetCallInterfaceDescriptor(); + CallDescriptor::Flags flags = AdjustFrameStatesForCall(node); CallDescriptor* desc = Linkage::GetStubCallDescriptor( - isolate(), zone(), d, static_cast(p.arity() - 1), - FlagsForNode(node)); + isolate(), zone(), d, static_cast(p.arity() - 1), flags); Node* stub_code = jsgraph()->HeapConstant(stub.GetCode()); node->InsertInput(zone(), 0, stub_code); node->set_op(common()->Call(desc)); diff --git a/src/compiler/js-type-feedback.cc b/src/compiler/js-type-feedback.cc index 02c9677..04861ea 100644 --- a/src/compiler/js-type-feedback.cc +++ b/src/compiler/js-type-feedback.cc @@ -61,9 +61,6 @@ Reduction JSTypeFeedbackSpecializer::Reduce(Node* node) { // StoreProperty(o, "constant", v) => StoreNamed["constant"](o, v). Unique name = match.Value(); LanguageMode language_mode = OpParameter(node); - // StoreProperty has 2 frame state inputs, but StoreNamed only 1. - DCHECK_EQ(2, OperatorProperties::GetFrameStateInputCount(node->op())); - node->RemoveInput(NodeProperties::FirstFrameStateIndex(node) + 1); node->set_op( jsgraph()->javascript()->StoreNamed(language_mode, name, KEYED)); node->RemoveInput(1); diff --git a/src/compiler/operator-properties.cc b/src/compiler/operator-properties.cc index 0ee434c..7ea0405 100644 --- a/src/compiler/operator-properties.cc +++ b/src/compiler/operator-properties.cc @@ -59,17 +59,14 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) { // Misc operations case IrOpcode::kJSStackCheck: + case IrOpcode::kJSDeleteProperty: + return 1; - // Properties + // We record the frame state immediately before and immediately after + // every property access. case IrOpcode::kJSLoadNamed: case IrOpcode::kJSStoreNamed: case IrOpcode::kJSLoadProperty: - case IrOpcode::kJSDeleteProperty: - return 1; - - // StoreProperty provides a second frame state just before - // the operation. This is used to lazy-deoptimize a to-number - // conversion for typed arrays. case IrOpcode::kJSStoreProperty: return 2; diff --git a/test/unittests/compiler/js-type-feedback-unittest.cc b/test/unittests/compiler/js-type-feedback-unittest.cc index 08fe68a..46eb437 100644 --- a/test/unittests/compiler/js-type-feedback-unittest.cc +++ b/test/unittests/compiler/js-type-feedback-unittest.cc @@ -80,10 +80,13 @@ class JSTypeFeedbackTest : public TypedGraphTest { Unique name = Unique::CreateUninitialized( isolate()->factory()->NewStringFromAsciiChecked(string)); - Node* load = graph()->NewNode(javascript()->LoadNamed(name, feedback), - global, context); + const Operator* op = javascript()->LoadNamed(name, feedback); + Node* load = graph()->NewNode(op, global, context); if (FLAG_turbo_deoptimization) { - load->AppendInput(zone(), EmptyFrameState()); + for (int i = 0; i < OperatorProperties::GetFrameStateInputCount(op); + i++) { + load->AppendInput(zone(), EmptyFrameState()); + } } load->AppendInput(zone(), effect); load->AppendInput(zone(), control); diff --git a/test/unittests/compiler/js-typed-lowering-unittest.cc b/test/unittests/compiler/js-typed-lowering-unittest.cc index 9e7b1ee..55f23a6 100644 --- a/test/unittests/compiler/js-typed-lowering-unittest.cc +++ b/test/unittests/compiler/js-typed-lowering-unittest.cc @@ -659,9 +659,9 @@ TEST_F(JSTypedLoweringTest, JSLoadPropertyFromExternalTypedArray) { Node* context = UndefinedConstant(); Node* effect = graph()->start(); Node* control = graph()->start(); - Reduction r = - Reduce(graph()->NewNode(javascript()->LoadProperty(feedback), base, key, - context, EmptyFrameState(), effect, control)); + Reduction r = Reduce(graph()->NewNode(javascript()->LoadProperty(feedback), + base, key, context, EmptyFrameState(), + EmptyFrameState(), effect, control)); Matcher offset_matcher = element_size == 1 @@ -700,9 +700,9 @@ TEST_F(JSTypedLoweringTest, JSLoadPropertyFromExternalTypedArrayWithSafeKey) { Node* context = UndefinedConstant(); Node* effect = graph()->start(); Node* control = graph()->start(); - Reduction r = - Reduce(graph()->NewNode(javascript()->LoadProperty(feedback), base, key, - context, EmptyFrameState(), effect, control)); + Reduction r = Reduce(graph()->NewNode(javascript()->LoadProperty(feedback), + base, key, context, EmptyFrameState(), + EmptyFrameState(), effect, control)); ASSERT_TRUE(r.Changed()); EXPECT_THAT( -- 2.7.4