From e3a46bc766a97d7070f02f1fda67af335e575e84 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Thu, 26 Nov 2015 06:29:36 -0800 Subject: [PATCH] [compiler] Decouple ToObject from CreateWithContext. Decouple the implicit ToObject for with statements from the actual creation of the with context. This way we can handle/optimize those constructs separately. R=mstarzinger@chromium.org Review URL: https://codereview.chromium.org/1481753003 Cr-Commit-Position: refs/heads/master@{#32341} --- src/ast.h | 5 +- src/compiler/ast-graph-builder.cc | 3 +- src/compiler/js-typed-lowering.cc | 46 ++++++++----------- src/full-codegen/full-codegen.cc | 7 ++- src/runtime/runtime-scopes.cc | 7 +-- .../compiler/js-typed-lowering-unittest.cc | 17 ++++--- 6 files changed, 42 insertions(+), 43 deletions(-) diff --git a/src/ast.h b/src/ast.h index cb9fb6f559..0be9948f95 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1007,8 +1007,9 @@ class WithStatement final : public Statement { void set_statement(Statement* s) { statement_ = s; } void set_base_id(int id) { base_id_ = id; } - static int num_ids() { return parent_num_ids() + 1; } - BailoutId EntryId() const { return BailoutId(local_id(0)); } + static int num_ids() { return parent_num_ids() + 2; } + BailoutId ToObjectId() const { return BailoutId(local_id(0)); } + BailoutId EntryId() const { return BailoutId(local_id(1)); } protected: WithStatement(Zone* zone, Scope* scope, Expression* expression, diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 66c778b88a..f623f666ca 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -1199,8 +1199,9 @@ void AstGraphBuilder::VisitReturnStatement(ReturnStatement* stmt) { void AstGraphBuilder::VisitWithStatement(WithStatement* stmt) { VisitForValue(stmt->expression()); Node* value = environment()->Pop(); + Node* object = BuildToObject(value, stmt->ToObjectId()); const Operator* op = javascript()->CreateWithContext(); - Node* context = NewNode(op, value, GetFunctionClosureForContext()); + Node* context = NewNode(op, object, GetFunctionClosureForContext()); PrepareFrameState(context, stmt->EntryId()); VisitInScope(stmt->statement(), stmt->scope(), context); } diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index e0eac28b47..64a1da66f0 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/src/compiler/js-typed-lowering.cc @@ -1885,33 +1885,25 @@ Reduction JSTypedLowering::ReduceJSCreateFunctionContext(Node* node) { Reduction JSTypedLowering::ReduceJSCreateWithContext(Node* node) { DCHECK_EQ(IrOpcode::kJSCreateWithContext, node->opcode()); - Node* const input = NodeProperties::GetValueInput(node, 0); - Node* const closure = NodeProperties::GetValueInput(node, 1); - Type* const input_type = NodeProperties::GetType(input); - - // Use inline allocation for with contexts for regular objects. - if (input_type->Is(Type::Receiver())) { - // JSCreateWithContext(o:receiver, fun) - Node* const effect = NodeProperties::GetEffectInput(node); - Node* const control = NodeProperties::GetControlInput(node); - Node* const context = NodeProperties::GetContextInput(node); - Node* const load = graph()->NewNode( - simplified()->LoadField( - AccessBuilder::ForContextSlot(Context::GLOBAL_OBJECT_INDEX)), - context, effect, control); - AllocationBuilder a(jsgraph(), effect, control); - STATIC_ASSERT(Context::MIN_CONTEXT_SLOTS == 4); // Ensure fully covered. - a.AllocateArray(Context::MIN_CONTEXT_SLOTS, factory()->with_context_map()); - a.Store(AccessBuilder::ForContextSlot(Context::CLOSURE_INDEX), closure); - a.Store(AccessBuilder::ForContextSlot(Context::PREVIOUS_INDEX), context); - a.Store(AccessBuilder::ForContextSlot(Context::EXTENSION_INDEX), input); - a.Store(AccessBuilder::ForContextSlot(Context::GLOBAL_OBJECT_INDEX), load); - RelaxControls(node); - a.FinishAndChange(node); - return Changed(node); - } - - return NoChange(); + Node* object = NodeProperties::GetValueInput(node, 0); + Node* closure = NodeProperties::GetValueInput(node, 1); + Node* effect = NodeProperties::GetEffectInput(node); + Node* control = NodeProperties::GetControlInput(node); + Node* context = NodeProperties::GetContextInput(node); + Node* global = effect = graph()->NewNode( + simplified()->LoadField( + AccessBuilder::ForContextSlot(Context::GLOBAL_OBJECT_INDEX)), + context, effect, control); + AllocationBuilder a(jsgraph(), effect, control); + STATIC_ASSERT(Context::MIN_CONTEXT_SLOTS == 4); // Ensure fully covered. + a.AllocateArray(Context::MIN_CONTEXT_SLOTS, factory()->with_context_map()); + a.Store(AccessBuilder::ForContextSlot(Context::CLOSURE_INDEX), closure); + a.Store(AccessBuilder::ForContextSlot(Context::PREVIOUS_INDEX), context); + a.Store(AccessBuilder::ForContextSlot(Context::EXTENSION_INDEX), object); + a.Store(AccessBuilder::ForContextSlot(Context::GLOBAL_OBJECT_INDEX), global); + RelaxControls(node); + a.FinishAndChange(node); + return Changed(node); } diff --git a/src/full-codegen/full-codegen.cc b/src/full-codegen/full-codegen.cc index c4d96454c4..03f1b51378 100644 --- a/src/full-codegen/full-codegen.cc +++ b/src/full-codegen/full-codegen.cc @@ -952,7 +952,12 @@ void FullCodeGenerator::VisitWithStatement(WithStatement* stmt) { Comment cmnt(masm_, "[ WithStatement"); SetStatementPosition(stmt); - VisitForStackValue(stmt->expression()); + VisitForAccumulatorValue(stmt->expression()); + Callable callable = CodeFactory::ToObject(isolate()); + __ Move(callable.descriptor().GetRegisterParameter(0), result_register()); + __ Call(callable.code(), RelocInfo::CODE_TARGET); + PrepareForBailoutForId(stmt->ToObjectId(), NO_REGISTERS); + __ Push(result_register()); PushFunctionArgumentForContextAllocation(); __ CallRuntime(Runtime::kPushWithContext, 2); StoreToFrameField(StandardFrameConstants::kContextOffset, context_register()); diff --git a/src/runtime/runtime-scopes.cc b/src/runtime/runtime-scopes.cc index 84e0937c09..14439fa387 100644 --- a/src/runtime/runtime-scopes.cc +++ b/src/runtime/runtime-scopes.cc @@ -706,13 +706,8 @@ RUNTIME_FUNCTION(Runtime_NewFunctionContext) { RUNTIME_FUNCTION(Runtime_PushWithContext) { HandleScope scope(isolate); DCHECK_EQ(2, args.length()); - CONVERT_ARG_HANDLE_CHECKED(Object, value, 0); + CONVERT_ARG_HANDLE_CHECKED(JSReceiver, extension_object, 0); CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 1); - Handle extension_object; - if (!Object::ToObject(isolate, value).ToHandle(&extension_object)) { - THROW_NEW_ERROR_RETURN_FAILURE( - isolate, NewTypeError(MessageTemplate::kUndefinedOrNullToObject)); - } Handle current(isolate->context()); Handle context = isolate->factory()->NewWithContext(function, current, extension_object); diff --git a/test/unittests/compiler/js-typed-lowering-unittest.cc b/test/unittests/compiler/js-typed-lowering-unittest.cc index 98b3f09741..e2a0a07539 100644 --- a/test/unittests/compiler/js-typed-lowering-unittest.cc +++ b/test/unittests/compiler/js-typed-lowering-unittest.cc @@ -1186,7 +1186,7 @@ TEST_F(JSTypedLoweringTest, JSCreateFunctionContextViaStub) { TEST_F(JSTypedLoweringTest, JSCreateWithContext) { Node* const object = Parameter(Type::Receiver()); - Node* const closure = Parameter(Type::Any()); + Node* const closure = Parameter(Type::Function()); Node* const context = Parameter(Type::Any()); Node* const frame_state = EmptyFrameState(); Node* const effect = graph()->start(); @@ -1195,11 +1195,16 @@ TEST_F(JSTypedLoweringTest, JSCreateWithContext) { Reduce(graph()->NewNode(javascript()->CreateWithContext(), object, closure, context, frame_state, effect, control)); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(r.replacement(), - IsFinishRegion(IsAllocate(IsNumberConstant(Context::SizeFor( - Context::MIN_CONTEXT_SLOTS)), - IsBeginRegion(effect), control), - _)); + EXPECT_THAT( + r.replacement(), + IsFinishRegion( + IsAllocate( + IsNumberConstant(Context::SizeFor(Context::MIN_CONTEXT_SLOTS)), + IsBeginRegion(IsLoadField( + AccessBuilder::ForContextSlot(Context::GLOBAL_OBJECT_INDEX), + context, effect, control)), + control), + _)); }