From 9d3c4b4b911ec2d0f456c1834a2d45f87c4d9dcf Mon Sep 17 00:00:00 2001 From: Michael Starzinger Date: Wed, 27 Sep 2017 10:06:47 +0200 Subject: [PATCH] [turbofan] Implement lowering of {JSCreateClosure}. This adds support for inline allocation of {JSFunction} objects as part of closures instantiation for {JSCreateClosure} nodes. The lowering is limited to instantiation sites which have already seen more than one previous instantiation, this avoids the need to increment the respective counter. R=jarin@chromium.org Change-Id: I462c557453fe58bc5f09020a3d5ebdf11c2ea68b Reviewed-on: https://chromium-review.googlesource.com/594287 Commit-Queue: Michael Starzinger Reviewed-by: Jaroslav Sevcik Cr-Commit-Position: refs/heads/master@{#48176} --- src/accessors.cc | 131 ++++++++++++++++------- src/compiler/js-create-lowering.cc | 44 ++++++++ src/compiler/js-create-lowering.h | 1 + src/deoptimizer.cc | 63 +++++++++-- src/deoptimizer.h | 1 + test/mjsunit/compiler/deopt-closure.js | 47 ++++++++ test/mjsunit/compiler/function-caller.js | 25 +++++ 7 files changed, 266 insertions(+), 46 deletions(-) create mode 100644 test/mjsunit/compiler/deopt-closure.js create mode 100644 test/mjsunit/compiler/function-caller.js diff --git a/src/accessors.cc b/src/accessors.cc index e61cdeada4..de641642ff 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -933,47 +933,99 @@ static inline bool AllowAccessToFunction(Context* current_context, class FrameFunctionIterator { public: explicit FrameFunctionIterator(Isolate* isolate) - : isolate_(isolate), frame_iterator_(isolate) { + : isolate_(isolate), frame_iterator_(isolate), inlined_frame_index_(-1) { GetFrames(); } + + // Iterate through functions until the first occurrence of 'function'. + // Returns true if one is found, and false if the iterator ends before. + bool Find(Handle function) { + do { + if (!next().ToHandle(&function_)) return false; + } while (!function_.is_identical_to(function)); + return true; + } + + // Iterate through functions until the next non-toplevel one is found. + // Returns true if one is found, and false if the iterator ends before. + bool FindNextNonTopLevel() { + do { + if (!next().ToHandle(&function_)) return false; + } while (function_->shared()->is_toplevel()); + return true; + } + + // Iterate through function until the first native or user-provided function + // is found. Functions not defined in user-provided scripts are not visible + // unless directly exposed, in which case the native flag is set on them. + // Returns true if one is found, and false if the iterator ends before. + bool FindFirstNativeOrUserJavaScript() { + while (!function_->shared()->native() && + !function_->shared()->IsUserJavaScript()) { + if (!next().ToHandle(&function_)) return false; + } + return true; + } + + // In case of inlined frames the function could have been materialized from + // deoptimization information. If that is the case we need to make sure that + // subsequent call will see the same function, since we are about to hand out + // the value to JavaScript. Make sure to store the materialized value and + // trigger a deoptimization of the underlying frame. + Handle MaterializeFunction() { + if (inlined_frame_index_ == 0) return function_; + + JavaScriptFrame* frame = frame_iterator_.frame(); + TranslatedState translated_values(frame); + translated_values.Prepare(frame->fp()); + + TranslatedFrame* translated_frame = + translated_values.GetFrameFromJSFrameIndex(inlined_frame_index_); + TranslatedFrame::iterator iter = translated_frame->begin(); + + // First value is the function. + bool should_deoptimize = iter->IsMaterializedObject(); + Handle value = iter->GetValue(); + if (should_deoptimize) { + translated_values.StoreMaterializedValuesAndDeopt(frame); + } + + return Handle::cast(value); + } + + private: MaybeHandle next() { while (true) { - if (frames_.empty()) return MaybeHandle(); - Handle next_function = - frames_.back().AsJavaScript().function(); - frames_.pop_back(); - if (frames_.empty()) { - GetFrames(); + inlined_frame_index_--; + if (inlined_frame_index_ == -1) { + if (!frame_iterator_.done()) { + frame_iterator_.Advance(); + frames_.clear(); + GetFrames(); + } + if (inlined_frame_index_ == -1) return MaybeHandle(); + inlined_frame_index_--; } + Handle next_function = + frames_[inlined_frame_index_].AsJavaScript().function(); // Skip functions from other origins. if (!AllowAccessToFunction(isolate_->context(), *next_function)) continue; return next_function; } } - - // Iterate through functions until the first occurrence of 'function'. - // Returns true if 'function' is found, and false if the iterator ends - // without finding it. - bool Find(Handle function) { - Handle next_function; - do { - if (!next().ToHandle(&next_function)) return false; - } while (!next_function.is_identical_to(function)); - return true; - } - - private: void GetFrames() { - DCHECK(frames_.empty()); + DCHECK_EQ(-1, inlined_frame_index_); if (frame_iterator_.done()) return; JavaScriptFrame* frame = frame_iterator_.frame(); frame->Summarize(&frames_); - DCHECK(!frames_.empty()); - frame_iterator_.Advance(); + inlined_frame_index_ = static_cast(frames_.size()); + DCHECK_LT(0, inlined_frame_index_); } Isolate* isolate_; + Handle function_; JavaScriptFrameIterator frame_iterator_; std::vector frames_; + int inlined_frame_index_; }; @@ -983,28 +1035,27 @@ MaybeHandle FindCaller(Isolate* isolate, if (function->shared()->native()) { return MaybeHandle(); } - // Find the function from the frames. + // Find the function from the frames. Return null in case no frame + // corresponding to the given function was found. if (!it.Find(function)) { - // No frame corresponding to the given function found. Return null. return MaybeHandle(); } // Find previously called non-toplevel function. - Handle caller; - do { - if (!it.next().ToHandle(&caller)) return MaybeHandle(); - } while (caller->shared()->is_toplevel()); + if (!it.FindNextNonTopLevel()) { + return MaybeHandle(); + } + // Find the first user-land JavaScript function (or the entry point into + // native JavaScript builtins in case such a builtin was the caller). + if (!it.FindFirstNativeOrUserJavaScript()) { + return MaybeHandle(); + } + + // Materialize the function that the iterator is currently sitting on. Note + // that this might trigger deoptimization in case the function was actually + // materialized. Identity of the function must be preserved because we are + // going to return it to JavaScript after this point. + Handle caller = it.MaterializeFunction(); - // If caller is not user code and caller's caller is also not user code, - // use that instead. - MaybeHandle potential_caller = caller; - while (!potential_caller.is_null() && - !potential_caller.ToHandleChecked()->shared()->IsUserJavaScript()) { - caller = potential_caller.ToHandleChecked(); - potential_caller = it.next(); - } - if (!caller->shared()->native() && !potential_caller.is_null()) { - caller = potential_caller.ToHandleChecked(); - } // Censor if the caller is not a sloppy mode function. // Change from ES5, which used to throw, see: // https://bugs.ecmascript.org/show_bug.cgi?id=310 diff --git a/src/compiler/js-create-lowering.cc b/src/compiler/js-create-lowering.cc index 6e6a5c57ed..d413c43d68 100644 --- a/src/compiler/js-create-lowering.cc +++ b/src/compiler/js-create-lowering.cc @@ -214,6 +214,8 @@ Reduction JSCreateLowering::Reduce(Node* node) { return ReduceJSCreateArguments(node); case IrOpcode::kJSCreateArray: return ReduceJSCreateArray(node); + case IrOpcode::kJSCreateClosure: + return ReduceJSCreateClosure(node); case IrOpcode::kJSCreateIterResultObject: return ReduceJSCreateIterResultObject(node); case IrOpcode::kJSCreateKeyValueArray: @@ -793,6 +795,48 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) { return ReduceNewArrayToStubCall(node, site); } +Reduction JSCreateLowering::ReduceJSCreateClosure(Node* node) { + DCHECK_EQ(IrOpcode::kJSCreateClosure, node->opcode()); + CreateClosureParameters const& p = CreateClosureParametersOf(node->op()); + Handle shared = p.shared_info(); + Node* effect = NodeProperties::GetEffectInput(node); + Node* control = NodeProperties::GetControlInput(node); + Node* context = NodeProperties::GetContextInput(node); + + // Use inline allocation of closures only for instantiation sites that have + // seen more than one instantiation, this simplifies the generated code and + // also serves as a heuristic of which allocation sites benefit from it. + FeedbackSlot slot(FeedbackVector::ToSlot(p.feedback().index())); + Handle vector_cell(Cell::cast(p.feedback().vector()->Get(slot))); + if (vector_cell->map() == isolate()->heap()->many_closures_cell_map()) { + Node* function_map = jsgraph()->HeapConstant( + handle(Map::cast(native_context()->get(shared->function_map_index())))); + Node* lazy_compile_builtin = jsgraph()->HeapConstant( + handle(isolate()->builtins()->builtin(Builtins::kCompileLazy))); + + // Emit code to allocate the JSFunction instance. + AllocationBuilder a(jsgraph(), effect, control); + a.Allocate(JSFunction::kSize); + a.Store(AccessBuilder::ForMap(), function_map); + a.Store(AccessBuilder::ForJSObjectPropertiesOrHash(), + jsgraph()->EmptyFixedArrayConstant()); + a.Store(AccessBuilder::ForJSObjectElements(), + jsgraph()->EmptyFixedArrayConstant()); + a.Store(AccessBuilder::ForJSFunctionPrototypeOrInitialMap(), + jsgraph()->TheHoleConstant()); + a.Store(AccessBuilder::ForJSFunctionSharedFunctionInfo(), shared); + a.Store(AccessBuilder::ForJSFunctionContext(), context); + a.Store(AccessBuilder::ForJSFunctionFeedbackVector(), vector_cell); + a.Store(AccessBuilder::ForJSFunctionCode(), lazy_compile_builtin); + STATIC_ASSERT(JSFunction::kSize == 8 * kPointerSize); + RelaxControls(node); + a.FinishAndChange(node); + return Changed(node); + } + + return NoChange(); +} + Reduction JSCreateLowering::ReduceJSCreateIterResultObject(Node* node) { DCHECK_EQ(IrOpcode::kJSCreateIterResultObject, node->opcode()); Node* value = NodeProperties::GetValueInput(node, 0); diff --git a/src/compiler/js-create-lowering.h b/src/compiler/js-create-lowering.h index 307e054a75..e8dc99c627 100644 --- a/src/compiler/js-create-lowering.h +++ b/src/compiler/js-create-lowering.h @@ -50,6 +50,7 @@ class V8_EXPORT_PRIVATE JSCreateLowering final Reduction ReduceJSCreate(Node* node); Reduction ReduceJSCreateArguments(Node* node); Reduction ReduceJSCreateArray(Node* node); + Reduction ReduceJSCreateClosure(Node* node); Reduction ReduceJSCreateIterResultObject(Node* node); Reduction ReduceJSCreateKeyValueArray(Node* node); Reduction ReduceJSCreateLiteralArrayOrObject(Node* node); diff --git a/src/deoptimizer.cc b/src/deoptimizer.cc index 289735ded7..1da2c6813d 100644 --- a/src/deoptimizer.cc +++ b/src/deoptimizer.cc @@ -573,9 +573,8 @@ int LookupCatchHandler(TranslatedFrame* translated_frame, int* data_out) { switch (translated_frame->kind()) { case TranslatedFrame::kInterpretedFunction: { int bytecode_offset = translated_frame->node_id().ToInt(); - JSFunction* function = - JSFunction::cast(translated_frame->begin()->GetRawValue()); - BytecodeArray* bytecode = function->shared()->bytecode_array(); + BytecodeArray* bytecode = + translated_frame->raw_shared_info()->bytecode_array(); HandlerTable* table = HandlerTable::cast(bytecode->handler_table()); return table->LookupRange(bytecode_offset, data_out, nullptr); } @@ -747,7 +746,8 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame, // add to the frame height here. if (is_topmost) height_in_bytes += kPointerSize; - JSFunction* function = JSFunction::cast(value_iterator->GetRawValue()); + TranslatedFrame::iterator function_iterator = value_iterator; + Object* function = value_iterator->GetRawValue(); value_iterator++; input_index++; if (trace_scope_ != NULL) { @@ -887,6 +887,12 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame, output_offset -= kPointerSize; value = reinterpret_cast(function); WriteValueToOutput(function, 0, frame_index, output_offset, "function "); + if (function == isolate_->heap()->arguments_marker()) { + Address output_address = + reinterpret_cast
(output_[frame_index]->GetTop()) + + output_offset; + values_to_materialize_.push_back({output_address, function_iterator}); + } // Set the bytecode array pointer. output_offset -= kPointerSize; @@ -1013,7 +1019,8 @@ void Deoptimizer::DoComputeArgumentsAdaptorFrame( unsigned height = translated_frame->height(); unsigned height_in_bytes = height * kPointerSize; - JSFunction* function = JSFunction::cast(value_iterator->GetRawValue()); + TranslatedFrame::iterator function_iterator = value_iterator; + Object* function = value_iterator->GetRawValue(); value_iterator++; input_index++; if (trace_scope_ != NULL) { @@ -1099,6 +1106,12 @@ void Deoptimizer::DoComputeArgumentsAdaptorFrame( output_offset -= kPointerSize; value = reinterpret_cast(function); WriteValueToOutput(function, 0, frame_index, output_offset, "function "); + if (function == isolate_->heap()->arguments_marker()) { + Address output_address = + reinterpret_cast
(output_[frame_index]->GetTop()) + + output_offset; + values_to_materialize_.push_back({output_address, function_iterator}); + } // Number of incoming arguments. output_offset -= kPointerSize; @@ -3613,6 +3626,32 @@ Handle TranslatedState::MaterializeCapturedObjectAt( object->set_bound_arguments(FixedArray::cast(*bound_arguments)); return object; } + case JS_FUNCTION_TYPE: { + Handle object = + isolate_->factory()->NewFunctionFromSharedFunctionInfo( + handle(isolate_->object_function()->shared()), + handle(isolate_->context()), NOT_TENURED); + slot->value_ = object; + // We temporarily allocated a JSFunction for the {Object} function + // within the current context, to break cycles in the object graph. + // The correct function and context will be set below once available. + Handle properties = materializer.FieldAt(value_index); + Handle elements = materializer.FieldAt(value_index); + Handle prototype = materializer.FieldAt(value_index); + Handle shared = materializer.FieldAt(value_index); + Handle context = materializer.FieldAt(value_index); + Handle vector_cell = materializer.FieldAt(value_index); + Handle code = materializer.FieldAt(value_index); + object->set_map(*map); + object->set_raw_properties_or_hash(*properties); + object->set_elements(FixedArrayBase::cast(*elements)); + object->set_prototype_or_initial_map(*prototype); + object->set_shared(SharedFunctionInfo::cast(*shared)); + object->set_context(Context::cast(*context)); + object->set_feedback_vector_cell(Cell::cast(*vector_cell)); + object->ReplaceCode(Code::cast(*code)); + return object; + } case JS_GENERATOR_OBJECT_TYPE: { Handle object = Handle::cast( isolate_->factory()->NewJSObjectFromMap(map, NOT_TENURED)); @@ -3774,7 +3813,6 @@ Handle TranslatedState::MaterializeCapturedObjectAt( case JS_API_OBJECT_TYPE: case JS_SPECIAL_API_OBJECT_TYPE: case JS_VALUE_TYPE: - case JS_FUNCTION_TYPE: case JS_MESSAGE_OBJECT_TYPE: case JS_DATE_TYPE: case JS_CONTEXT_EXTENSION_OBJECT_TYPE: @@ -3908,6 +3946,19 @@ Handle TranslatedState::MaterializeObjectAt(int object_index) { return MaterializeAt(pos.frame_index_, &(pos.value_index_)); } +TranslatedFrame* TranslatedState::GetFrameFromJSFrameIndex(int jsframe_index) { + for (size_t i = 0; i < frames_.size(); i++) { + if (frames_[i].kind() == TranslatedFrame::kInterpretedFunction) { + if (jsframe_index > 0) { + jsframe_index--; + } else { + return &(frames_[i]); + } + } + } + return nullptr; +} + TranslatedFrame* TranslatedState::GetArgumentsInfoFromJSFrameIndex( int jsframe_index, int* args_count) { for (size_t i = 0; i < frames_.size(); i++) { diff --git a/src/deoptimizer.h b/src/deoptimizer.h index 906164aeef..6d069fc0a0 100644 --- a/src/deoptimizer.h +++ b/src/deoptimizer.h @@ -265,6 +265,7 @@ class TranslatedState { std::vector& frames() { return frames_; } + TranslatedFrame* GetFrameFromJSFrameIndex(int jsframe_index); TranslatedFrame* GetArgumentsInfoFromJSFrameIndex(int jsframe_index, int* arguments_count); diff --git a/test/mjsunit/compiler/deopt-closure.js b/test/mjsunit/compiler/deopt-closure.js new file mode 100644 index 0000000000..2ce531faf0 --- /dev/null +++ b/test/mjsunit/compiler/deopt-closure.js @@ -0,0 +1,47 @@ +// Copyright 2017 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +(function TestMaterializeTargetOfInterpretedFrame() { + function f(x) { + function g() { + %_DeoptimizeNow(); + return x + 1; + } + return g(); + } + assertEquals(24, f(23)); + assertEquals(43, f(42)); + %OptimizeFunctionOnNextCall(f); + assertEquals(66, f(65)); +})(); + +(function TestMaterializeTargetOfArgumentsAdaptorFrame() { + function f(x) { + function g(a, b, c) { + %_DeoptimizeNow(); + return x + 1; + } + return g(); + } + assertEquals(24, f(23)); + assertEquals(43, f(42)); + %OptimizeFunctionOnNextCall(f); + assertEquals(66, f(65)); +})(); + +(function TestMaterializeTargetOfConstructStubFrame() { + function f(x) { + function g() { + %_DeoptimizeNow(); + this.val = x + 1; + } + return new g(); + } + assertEquals({ val: 24 }, f(23)); + assertEquals({ val: 43 }, f(42)); + %OptimizeFunctionOnNextCall(f); + assertEquals({ val: 66 }, f(65)); +})(); diff --git a/test/mjsunit/compiler/function-caller.js b/test/mjsunit/compiler/function-caller.js new file mode 100644 index 0000000000..1192e680cb --- /dev/null +++ b/test/mjsunit/compiler/function-caller.js @@ -0,0 +1,25 @@ +// Copyright 2017 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +(function TestInlineAllocatedCaller() { + function g() { + var caller = g.caller; + caller.foo = 23; + assertEquals(23, caller.foo); + assertEquals(23, g.caller.foo); + assertSame(caller, g.caller); + } + %NeverOptimizeFunction(g); + + function f() { + (function caller() { g() })(); + } + + f(); + f(); + %OptimizeFunctionOnNextCall(f); + f(); +})();