From 69c8f16da7a9587d68904af36eb6ccac8ab5b1eb Mon Sep 17 00:00:00 2001 From: Alexandre Talon Date: Wed, 19 Jul 2017 18:16:59 +0100 Subject: [PATCH] [Turbofan] Merged the OSR phase into the graph building phase. Now the OSR phase is only used when OSRing from the ast graph builder. When OSRing from Turbofan, the implementation is now in the graph building phase, at the beginning of the VisitBytecode function. We are no longer generating any OSRLoopEntry or OSRNormalEntry nodes, nor nodes for the possible code of the OSRed function which is before the OSRed loops. The trimming and reducing of the OSR phase is not done either. This change in the way the way the OSR is done enabled to remove the workaround to the bug mentioned below. Bug: v8:6112 Bug: v8:6518 Change-Id: I1c9231810b923486d55ea618d550d981d695d797 Reviewed-on: https://chromium-review.googlesource.com/543042 Commit-Queue: Alexandre Talon Reviewed-by: Michael Starzinger Reviewed-by: Ross McIlroy Cr-Commit-Position: refs/heads/master@{#46801} --- src/compiler/bytecode-analysis.h | 5 +- src/compiler/bytecode-graph-builder.cc | 340 ++++++++++++++++++------- src/compiler/bytecode-graph-builder.h | 43 +++- src/compiler/js-inlining.cc | 5 +- src/compiler/osr.cc | 5 +- src/compiler/osr.h | 7 +- src/compiler/pipeline.cc | 10 +- src/source-position-table.h | 13 + test/mjsunit/compiler/osr-try.js | 23 ++ 9 files changed, 334 insertions(+), 117 deletions(-) create mode 100644 test/mjsunit/compiler/osr-try.js diff --git a/src/compiler/bytecode-analysis.h b/src/compiler/bytecode-analysis.h index 68433a4155..35eb4ad12f 100644 --- a/src/compiler/bytecode-analysis.h +++ b/src/compiler/bytecode-analysis.h @@ -80,10 +80,9 @@ class V8_EXPORT_PRIVATE BytecodeAnalysis BASE_EMBEDDED { const LoopInfo& GetLoopInfoFor(int header_offset) const; // True if the current analysis has an OSR entry point. - bool HasOSREntryPoint() const { return osr_entry_point_ != -1; } - // True if {offset} is the OSR entry loop header. - bool IsOSREntryPoint(int offset) const { return osr_entry_point_ == offset; } + bool HasOsrEntryPoint() const { return osr_entry_point_ != -1; } + int osr_entry_point() const { return osr_entry_point_; } // Gets the in-liveness for the bytecode at {offset}. const BytecodeLivenessState* GetInLivenessFor(int offset) const; diff --git a/src/compiler/bytecode-graph-builder.cc b/src/compiler/bytecode-graph-builder.cc index b97555b9df..4264789836 100644 --- a/src/compiler/bytecode-graph-builder.cc +++ b/src/compiler/bytecode-graph-builder.cc @@ -71,7 +71,7 @@ class BytecodeGraphBuilder::Environment : public ZoneObject { Environment* Copy(); void Merge(Environment* other); - void PrepareForOsrEntry(); + void FillWithOsrValues(); void PrepareForLoop(const BytecodeLoopAssignments& assignments); void PrepareForLoopExit(Node* loop, const BytecodeLoopAssignments& assignments); @@ -307,27 +307,18 @@ void BytecodeGraphBuilder::Environment::PrepareForLoop( builder()->exit_controls_.push_back(terminate); } -void BytecodeGraphBuilder::Environment::PrepareForOsrEntry() { - DCHECK_EQ(IrOpcode::kLoop, GetControlDependency()->opcode()); - DCHECK_EQ(1, GetControlDependency()->InputCount()); - +void BytecodeGraphBuilder::Environment::FillWithOsrValues() { Node* start = graph()->start(); - // Create a control node for the OSR entry point and update the current - // environment's dependencies accordingly. - Node* entry = graph()->NewNode(common()->OsrLoopEntry(), start, start); - UpdateControlDependency(entry); - UpdateEffectDependency(entry); - // Create OSR values for each environment value. SetContext(graph()->NewNode( - common()->OsrValue(Linkage::kOsrContextSpillSlotIndex), entry)); + common()->OsrValue(Linkage::kOsrContextSpillSlotIndex), start)); int size = static_cast(values()->size()); for (int i = 0; i < size; i++) { int idx = i; // Indexing scheme follows {StandardFrame}, adapt accordingly. if (i >= register_base()) idx += InterpreterFrameConstants::kExtraSlotCount; if (i >= accumulator_base()) idx = Linkage::kOsrAccumulatorRegisterIndex; - values()->at(i) = graph()->NewNode(common()->OsrValue(idx), entry); + values()->at(i) = graph()->NewNode(common()->OsrValue(idx), start); } } @@ -440,7 +431,7 @@ BytecodeGraphBuilder::BytecodeGraphBuilder( Handle feedback_vector, BailoutId osr_ast_id, JSGraph* jsgraph, CallFrequency invocation_frequency, SourcePositionTable* source_positions, int inlining_id, - JSTypeHintLowering::Flags flags) + JSTypeHintLowering::Flags flags, bool stack_check) : local_zone_(local_zone), jsgraph_(jsgraph), invocation_frequency_(invocation_frequency), @@ -457,6 +448,8 @@ BytecodeGraphBuilder::BytecodeGraphBuilder( bytecode_analysis_(nullptr), environment_(nullptr), osr_ast_id_(osr_ast_id), + currently_peeled_loop_offset_(-1), + stack_check_(stack_check), merge_environments_(local_zone), exception_handlers_(local_zone), current_exception_handler_(0), @@ -521,7 +514,7 @@ VectorSlotPair BytecodeGraphBuilder::CreateVectorSlotPair(int slot_id) { return VectorSlotPair(feedback_vector(), slot); } -void BytecodeGraphBuilder::CreateGraph(bool stack_check) { +void BytecodeGraphBuilder::CreateGraph() { SourcePositionTable::Scope pos_scope(source_positions_, start_position_); // Set up the basic structure of the graph. Outputs for {Start} are the formal @@ -535,7 +528,7 @@ void BytecodeGraphBuilder::CreateGraph(bool stack_check) { GetFunctionContext()); set_environment(&env); - VisitBytecodes(stack_check); + VisitBytecodes(); // Finish the basic structure of the graph. DCHECK_NE(0u, exit_controls_.size()); @@ -600,7 +593,221 @@ void BytecodeGraphBuilder::PrepareFrameState(Node* node, } } -void BytecodeGraphBuilder::VisitBytecodes(bool stack_check) { +// Stores the state of the SourcePosition iterator, and the index to the +// current exception handlers stack. We need, during the OSR graph generation, +// to backup the states of these iterators at the LoopHeader offset of each +// outer loop which contains the OSR loop. The iterators are then restored when +// peeling the loops, so that both exception handling and synchronisation with +// the source position can be achieved. +class BytecodeGraphBuilder::OsrIteratorState { + public: + OsrIteratorState(interpreter::BytecodeArrayIterator* iterator, + SourcePositionTableIterator* source_position_iterator, + BytecodeGraphBuilder* graph_builder) + : iterator_(iterator), + source_position_iterator_(source_position_iterator), + graph_builder_(graph_builder), + saved_states_(graph_builder->local_zone()) {} + + void ProcessOsrPrelude() { + ZoneVector outer_loop_offsets(graph_builder_->local_zone()); + + const BytecodeAnalysis& bytecode_analysis = + *(graph_builder_->bytecode_analysis()); + int osr_offset = bytecode_analysis.osr_entry_point(); + + // We find here the outermost loop which contains the OSR loop. + int outermost_loop_offset = osr_offset; + while ((outermost_loop_offset = + bytecode_analysis.GetLoopInfoFor(outermost_loop_offset) + .parent_offset()) != -1) { + outer_loop_offsets.push_back(outermost_loop_offset); + } + outermost_loop_offset = + outer_loop_offsets.empty() ? osr_offset : outer_loop_offsets.back(); + + // We will not processs any bytecode before the outermost_loop_offset, but + // the source_position_iterator needs to be advanced step by step through + // the bytecode. + for (; iterator_->current_offset() != outermost_loop_offset; + iterator_->Advance()) { + graph_builder_->UpdateSourcePosition(source_position_iterator_, + iterator_->current_offset()); + } + + // We save some iterators states at the offsets of the loop headers of the + // outer loops (the ones containing the OSR loop). They will be used for + // jumping back in the bytecode. + for (ZoneVector::const_reverse_iterator it = + outer_loop_offsets.crbegin(); + it != outer_loop_offsets.crend(); ++it) { + int next_loop_offset = *it; + for (; iterator_->current_offset() != next_loop_offset; + iterator_->Advance()) { + graph_builder_->UpdateSourcePosition(source_position_iterator_, + iterator_->current_offset()); + graph_builder_->EnterAndExitExceptionHandlers( + iterator_->current_offset()); + } + saved_states_.push( + IteratorsStates(graph_builder_->current_exception_handler(), + source_position_iterator_->GetState())); + } + + // Finishing by advancing to the OSR entry + for (; iterator_->current_offset() != osr_offset; iterator_->Advance()) { + graph_builder_->UpdateSourcePosition(source_position_iterator_, + iterator_->current_offset()); + } + + graph_builder_->set_currently_peeled_loop_offset( + bytecode_analysis.GetLoopInfoFor(osr_offset).parent_offset()); + } + + void RestoreState(int target_offset, int new_parent_offset) { + iterator_->SetOffset(target_offset); + // In case of a return, we must not build loop exits for + // not-yet-built outer loops. + graph_builder_->set_currently_peeled_loop_offset(new_parent_offset); + IteratorsStates saved_state = saved_states_.top(); + source_position_iterator_->RestoreState(saved_state.source_iterator_state_); + graph_builder_->set_current_exception_handler( + saved_state.exception_handler_index_); + saved_states_.pop(); + } + + private: + struct IteratorsStates { + int exception_handler_index_; + SourcePositionTableIterator::IndexAndPosition source_iterator_state_; + + IteratorsStates( + int exception_handler_index, + SourcePositionTableIterator::IndexAndPosition source_iterator_state) + : exception_handler_index_(exception_handler_index), + source_iterator_state_(source_iterator_state) {} + }; + + interpreter::BytecodeArrayIterator* iterator_; + SourcePositionTableIterator* source_position_iterator_; + BytecodeGraphBuilder* graph_builder_; + ZoneStack saved_states_; +}; + +void BytecodeGraphBuilder::RemoveMergeEnvironmentsBeforeOffset( + int limit_offset) { + if (!merge_environments_.empty()) { + ZoneMap::iterator it = merge_environments_.begin(); + ZoneMap::iterator stop_it = merge_environments_.end(); + while (it != stop_it && it->first <= limit_offset) { + it = merge_environments_.erase(it); + } + } +} + +// We will iterate through the OSR loop, then its parent, and so on +// until we have reached the outmost loop containing the OSR loop. We do +// not generate nodes for anything before the outermost loop. +void BytecodeGraphBuilder::AdvanceToOsrEntryAndPeelLoops( + interpreter::BytecodeArrayIterator* iterator, + SourcePositionTableIterator* source_position_iterator) { + const BytecodeAnalysis& analysis = *(bytecode_analysis()); + int osr_offset = analysis.osr_entry_point(); + OsrIteratorState iterator_states(iterator, source_position_iterator, this); + + iterator_states.ProcessOsrPrelude(); + DCHECK_EQ(iterator->current_offset(), osr_offset); + + environment()->FillWithOsrValues(); + + // Suppose we have n nested loops, loop_0 being the outermost one, and + // loop_n being the OSR loop. We start iterating the bytecode at the header + // of loop_n (the OSR loop), and then we peel the part of the the body of + // loop_{n-1} following the end of loop_n. We then rewind the iterator to + // the header of loop_{n-1}, and so on until we have partly peeled loop 0. + // The full loop_0 body will be generating with the rest of the function, + // outside the OSR generation. + + // To do so, if we are visiting a loop, we continue to visit what's left + // of its parent, and then when reaching the parent's JumpLoop, we do not + // create any jump for that but rewind the bytecode iterator to visit the + // parent loop entirely, and so on. + + int current_parent_offset = + analysis.GetLoopInfoFor(osr_offset).parent_offset(); + while (current_parent_offset != -1) { + LoopInfo current_parent_loop = + analysis.GetLoopInfoFor(current_parent_offset); + // We iterate until the back edge of the parent loop, which we detect by + // the offset that the JumpLoop targets. + for (; !iterator->done(); iterator->Advance()) { + if (iterator->current_bytecode() == interpreter::Bytecode::kJumpLoop && + iterator->GetJumpTargetOffset() == current_parent_offset) { + // Reached the end of the current parent loop. + break; + } + VisitSingleBytecode(source_position_iterator); + } + DCHECK(!iterator->done()); // Should have found the loop's jump target. + + // We also need to take care of the merge environments and exceptions + // handlers here because the omitted JumpLoop bytecode can still be the + // target of jumps or the first bytecode after a try block. + EnterAndExitExceptionHandlers(iterator->current_offset()); + SwitchToMergeEnvironment(iterator->current_offset()); + + // This jump is the jump of our parent loop, which is not yet created. + // So we do not build the jump nodes, but restore the bytecode and the + // SourcePosition iterators to the values they had when we were visiting + // the offset pointed at by the JumpLoop we've just reached. + // We have already built nodes for inner loops, but now we will + // iterate again over them and build new nodes corresponding to the same + // bytecode offsets. Any jump or reference to this inner loops must now + // point to the new nodes we will build, hence we clear the relevant part + // of the environment. + // Completely clearing the environment is not possible because merge + // environments for forward jumps out of the loop need to be preserved + // (e.g. a return or a labeled break in the middle of a loop). + RemoveMergeEnvironmentsBeforeOffset(iterator->current_offset()); + iterator_states.RestoreState(current_parent_offset, + current_parent_loop.parent_offset()); + current_parent_offset = current_parent_loop.parent_offset(); + } +} + +// TODO(alexandret) create an iterator which wraps both bytecode_iterator and +// source_position_iterator and increments them in one, so we don't need to +// pass source_position_iterator as an argument here. +void BytecodeGraphBuilder::VisitSingleBytecode( + SourcePositionTableIterator* source_position_iterator) { + const interpreter::BytecodeArrayIterator& iterator = bytecode_iterator(); + int current_offset = iterator.current_offset(); + UpdateSourcePosition(source_position_iterator, current_offset); + EnterAndExitExceptionHandlers(current_offset); + SwitchToMergeEnvironment(current_offset); + + if (environment() != nullptr) { + BuildLoopHeaderEnvironment(current_offset); + + // Skip the first stack check if stack_check is false + if (!stack_check() && + iterator.current_bytecode() == interpreter::Bytecode::kStackCheck) { + set_stack_check(true); + return; + } + + switch (iterator.current_bytecode()) { +#define BYTECODE_CASE(name, ...) \ + case interpreter::Bytecode::k##name: \ + Visit##name(); \ + break; + BYTECODE_LIST(BYTECODE_CASE) +#undef BYTECODE_CODE + } + } +} + +void BytecodeGraphBuilder::VisitBytecodes() { BytecodeAnalysis bytecode_analysis(bytecode_array(), local_zone(), FLAG_analyze_environment_liveness); bytecode_analysis.Analyze(osr_ast_id_); @@ -617,32 +824,16 @@ void BytecodeGraphBuilder::VisitBytecodes(bool stack_check) { bytecode_analysis.PrintLivenessTo(of); } - BuildOSRNormalEntryPoint(); + if (bytecode_analysis.HasOsrEntryPoint()) { + // We peel the OSR loop and any outer loop containing it except that we + // leave the nodes corresponding to the whole outermost loop (including + // the last copies of the loops it contains) to be generated by the normal + // bytecode iteration below. + AdvanceToOsrEntryAndPeelLoops(&iterator, &source_position_iterator); + } for (; !iterator.done(); iterator.Advance()) { - int current_offset = iterator.current_offset(); - UpdateCurrentSourcePosition(&source_position_iterator, current_offset); - EnterAndExitExceptionHandlers(current_offset); - SwitchToMergeEnvironment(current_offset); - if (environment() != nullptr) { - BuildLoopHeaderEnvironment(current_offset); - - // Skip the first stack check if stack_check is false - if (!stack_check && - iterator.current_bytecode() == interpreter::Bytecode::kStackCheck) { - stack_check = true; - continue; - } - - switch (iterator.current_bytecode()) { -#define BYTECODE_CASE(name, ...) \ - case interpreter::Bytecode::k##name: \ - Visit##name(); \ - break; - BYTECODE_LIST(BYTECODE_CASE) -#undef BYTECODE_CODE - } - } + VisitSingleBytecode(&source_position_iterator); } set_bytecode_analysis(nullptr); set_bytecode_iterator(nullptr); @@ -867,8 +1058,8 @@ BytecodeGraphBuilder::Environment* BytecodeGraphBuilder::CheckContextExtensions( // Output environment where the context has an extension Environment* slow_environment = nullptr; - // We only need to check up to the last-but-one depth, because the an eval in - // the same scope as the variable itself has no way of shadowing it. + // We only need to check up to the last-but-one depth, because the an eval + // in the same scope as the variable itself has no way of shadowing it. for (uint32_t d = 0; d < depth; d++) { Node* extension_slot = NewNode(javascript()->LoadContext(d, Context::EXTENSION_INDEX, false)); @@ -897,8 +1088,8 @@ BytecodeGraphBuilder::Environment* BytecodeGraphBuilder::CheckContextExtensions( // the fast path. } - // The depth can be zero, in which case no slow-path checks are built, and the - // slow path environment can be null. + // The depth can be zero, in which case no slow-path checks are built, and + // the slow path environment can be null. DCHECK(depth == 0 || slow_environment != nullptr); return slow_environment; @@ -2416,8 +2607,6 @@ void BytecodeGraphBuilder::BuildLoopHeaderEnvironment(int current_offset) { // Add loop header. environment()->PrepareForLoop(loop_info.assignments()); - BuildOSRLoopEntryPoint(current_offset); - // Store a copy of the environment so we can connect merged back edge inputs // to the loop header. merge_environments_[current_offset] = environment()->Copy(); @@ -2427,6 +2616,7 @@ void BytecodeGraphBuilder::BuildLoopHeaderEnvironment(int current_offset) { void BytecodeGraphBuilder::MergeIntoSuccessorEnvironment(int target_offset) { BuildLoopExitsForBranch(target_offset); Environment*& merge_environment = merge_environments_[target_offset]; + if (merge_environment == nullptr) { // Append merge nodes to the environment. We may merge here with another // environment. So add a place holder for merge nodes. We may add redundant @@ -2445,26 +2635,6 @@ void BytecodeGraphBuilder::MergeControlToLeaveFunction(Node* exit) { set_environment(nullptr); } -void BytecodeGraphBuilder::BuildOSRLoopEntryPoint(int current_offset) { - DCHECK(bytecode_analysis()->IsLoopHeader(current_offset)); - - if (bytecode_analysis()->IsOSREntryPoint(current_offset)) { - // For OSR add a special {OsrLoopEntry} node into the current loop header. - // It will be turned into a usable entry by the OSR deconstruction. - Environment* osr_env = environment()->Copy(); - osr_env->PrepareForOsrEntry(); - environment()->Merge(osr_env); - } -} - -void BytecodeGraphBuilder::BuildOSRNormalEntryPoint() { - if (bytecode_analysis()->HasOSREntryPoint()) { - // For OSR add an {OsrNormalEntry} as the the top-level environment start. - // It will be replaced with {Dead} by the OSR deconstruction. - NewNode(common()->OsrNormalEntry()); - } -} - void BytecodeGraphBuilder::BuildLoopExitsForBranch(int target_offset) { int origin_offset = bytecode_iterator().current_offset(); // Only build loop exits for forward edges. @@ -2477,6 +2647,10 @@ void BytecodeGraphBuilder::BuildLoopExitsForBranch(int target_offset) { void BytecodeGraphBuilder::BuildLoopExitsUntilLoop(int loop_offset) { int origin_offset = bytecode_iterator().current_offset(); int current_loop = bytecode_analysis()->GetLoopOffsetFor(origin_offset); + // The limit_offset is the stop offset for building loop exists, used for OSR. + // It prevents the creations of loopexits for loops which do not exist. + loop_offset = std::max(loop_offset, currently_peeled_loop_offset_); + while (loop_offset < current_loop) { Node* loop_node = merge_environments_[current_loop]->GetControlDependency(); const LoopInfo& loop_info = @@ -2610,10 +2784,6 @@ Node* BytecodeGraphBuilder::TryBuildSimplifiedCall(const Operator* op, Node* const* args, int arg_count, FeedbackSlot slot) { - // TODO(mstarzinger,6112): This is a workaround for OSR loop entries being - // pruned from the graph by a soft-deopt. It can happen that a CallIC that - // control-dominates the OSR entry is still in "uninitialized" state. - if (!osr_ast_id_.IsNone()) return nullptr; Node* effect = environment()->GetEffectDependency(); Node* control = environment()->GetControlDependency(); Reduction early_reduction = type_hint_lowering().ReduceCallOperation( @@ -2629,10 +2799,6 @@ Node* BytecodeGraphBuilder::TryBuildSimplifiedConstruct(const Operator* op, Node* const* args, int arg_count, FeedbackSlot slot) { - // TODO(mstarzinger,6112): This is a workaround for OSR loop entries being - // pruned from the graph by a soft-deopt. It can happen that a CallIC that - // control-dominates the OSR entry is still in "uninitialized" state. - if (!osr_ast_id_.IsNone()) return nullptr; Node* effect = environment()->GetEffectDependency(); Node* control = environment()->GetControlDependency(); Reduction early_reduction = type_hint_lowering().ReduceConstructOperation( @@ -2647,10 +2813,6 @@ Node* BytecodeGraphBuilder::TryBuildSimplifiedConstruct(const Operator* op, Node* BytecodeGraphBuilder::TryBuildSimplifiedLoadNamed(const Operator* op, Node* receiver, FeedbackSlot slot) { - // TODO(mstarzinger,6112): This is a workaround for OSR loop entries being - // pruned from the graph by a soft-deopt. It can happen that a LoadIC that - // control-dominates the OSR entry is still in "uninitialized" state. - if (bytecode_analysis()->HasOSREntryPoint()) return nullptr; Node* effect = environment()->GetEffectDependency(); Node* control = environment()->GetControlDependency(); Reduction early_reduction = type_hint_lowering().ReduceLoadNamedOperation( @@ -2666,10 +2828,6 @@ Node* BytecodeGraphBuilder::TryBuildSimplifiedLoadKeyed(const Operator* op, Node* receiver, Node* key, FeedbackSlot slot) { - // TODO(mstarzinger,6112): This is a workaround for OSR loop entries being - // pruned from the graph by a soft-deopt. It can happen that a LoadIC that - // control-dominates the OSR entry is still in "uninitialized" state. - if (bytecode_analysis()->HasOSREntryPoint()) return nullptr; Node* effect = environment()->GetEffectDependency(); Node* control = environment()->GetControlDependency(); Reduction early_reduction = type_hint_lowering().ReduceLoadKeyedOperation( @@ -2685,10 +2843,6 @@ Node* BytecodeGraphBuilder::TryBuildSimplifiedStoreNamed(const Operator* op, Node* receiver, Node* value, FeedbackSlot slot) { - // TODO(mstarzinger,6112): This is a workaround for OSR loop entries being - // pruned from the graph by a soft-deopt. It can happen that a LoadIC that - // control-dominates the OSR entry is still in "uninitialized" state. - if (bytecode_analysis()->HasOSREntryPoint()) return nullptr; Node* effect = environment()->GetEffectDependency(); Node* control = environment()->GetControlDependency(); Reduction early_reduction = type_hint_lowering().ReduceStoreNamedOperation( @@ -2704,10 +2858,6 @@ Node* BytecodeGraphBuilder::TryBuildSimplifiedStoreKeyed(const Operator* op, Node* receiver, Node* key, Node* value, FeedbackSlot slot) { - // TODO(mstarzinger,6112): This is a workaround for OSR loop entries being - // pruned from the graph by a soft-deopt. It can happen that a LoadIC that - // control-dominates the OSR entry is still in "uninitialized" state. - if (bytecode_analysis()->HasOSREntryPoint()) return nullptr; Node* effect = environment()->GetEffectDependency(); Node* control = environment()->GetControlDependency(); Reduction early_reduction = type_hint_lowering().ReduceStoreKeyedOperation( @@ -2854,7 +3004,6 @@ Node* BytecodeGraphBuilder::NewPhi(int count, Node* input, Node* control) { return graph()->NewNode(phi_op, count + 1, buffer, true); } - Node* BytecodeGraphBuilder::NewEffectPhi(int count, Node* input, Node* control) { const Operator* phi_op = common()->EffectPhi(count); @@ -2886,7 +3035,6 @@ Node* BytecodeGraphBuilder::MergeControl(Node* control, Node* other) { return control; } - Node* BytecodeGraphBuilder::MergeEffect(Node* value, Node* other, Node* control) { int inputs = control->op()->ControlInputCount(); @@ -2903,7 +3051,6 @@ Node* BytecodeGraphBuilder::MergeEffect(Node* value, Node* other, return value; } - Node* BytecodeGraphBuilder::MergeValue(Node* value, Node* other, Node* control) { int inputs = control->op()->ControlInputCount(); @@ -2921,10 +3068,9 @@ Node* BytecodeGraphBuilder::MergeValue(Node* value, Node* other, return value; } -void BytecodeGraphBuilder::UpdateCurrentSourcePosition( - SourcePositionTableIterator* it, int offset) { +void BytecodeGraphBuilder::UpdateSourcePosition(SourcePositionTableIterator* it, + int offset) { if (it->done()) return; - if (it->code_offset() == offset) { source_positions_->SetCurrentPosition(SourcePosition( it->source_position().ScriptOffset(), start_position_.InliningId())); diff --git a/src/compiler/bytecode-graph-builder.h b/src/compiler/bytecode-graph-builder.h index 312c58c16f..91bf3521d7 100644 --- a/src/compiler/bytecode-graph-builder.h +++ b/src/compiler/bytecode-graph-builder.h @@ -31,16 +31,25 @@ class BytecodeGraphBuilder { JSGraph* jsgraph, CallFrequency invocation_frequency, SourcePositionTable* source_positions, int inlining_id = SourcePosition::kNotInlined, - JSTypeHintLowering::Flags flags = JSTypeHintLowering::kNoFlags); + JSTypeHintLowering::Flags flags = JSTypeHintLowering::kNoFlags, + bool stack_check = true); // Creates a graph by visiting bytecodes. - void CreateGraph(bool stack_check = true); + void CreateGraph(); private: class Environment; + class OsrIteratorState; struct SubEnvironment; - void VisitBytecodes(bool stack_check); + void RemoveMergeEnvironmentsBeforeOffset(int limit_offset); + void AdvanceToOsrEntryAndPeelLoops( + interpreter::BytecodeArrayIterator* iterator, + SourcePositionTableIterator* source_position_iterator); + + void VisitSingleBytecode( + SourcePositionTableIterator* source_position_iterator); + void VisitBytecodes(); // Get or create the node that represents the outer function closure. Node* GetFunctionClosure(); @@ -236,10 +245,6 @@ class BytecodeGraphBuilder { // Simulates control flow that exits the function body. void MergeControlToLeaveFunction(Node* exit); - // Builds entry points that are used by OSR deconstruction. - void BuildOSRLoopEntryPoint(int current_offset); - void BuildOSRNormalEntryPoint(); - // Builds loop exit nodes for every exited loop between the current bytecode // offset and {target_offset}. void BuildLoopExitsForBranch(int target_offset); @@ -251,7 +256,7 @@ class BytecodeGraphBuilder { // Update the current position of the {SourcePositionTable} to that of the // bytecode at {offset}, if any. - void UpdateCurrentSourcePosition(SourcePositionTableIterator* it, int offset); + void UpdateSourcePosition(SourcePositionTableIterator* it, int offset); // Growth increment for the temporary buffer used to construct input lists to // new nodes. @@ -299,7 +304,7 @@ class BytecodeGraphBuilder { } void set_bytecode_iterator( - const interpreter::BytecodeArrayIterator* bytecode_iterator) { + interpreter::BytecodeArrayIterator* bytecode_iterator) { bytecode_iterator_ = bytecode_iterator; } @@ -311,6 +316,24 @@ class BytecodeGraphBuilder { bytecode_analysis_ = bytecode_analysis; } + int currently_peeled_loop_offset() const { + return currently_peeled_loop_offset_; + } + + void set_currently_peeled_loop_offset(int offset) { + currently_peeled_loop_offset_ = offset; + } + + bool stack_check() const { return stack_check_; } + + void set_stack_check(bool stack_check) { stack_check_ = stack_check; } + + int current_exception_handler() { return current_exception_handler_; } + + void set_current_exception_handler(int index) { + current_exception_handler_ = index; + } + bool needs_eager_checkpoint() const { return needs_eager_checkpoint_; } void mark_as_needing_eager_checkpoint(bool value) { needs_eager_checkpoint_ = value; @@ -332,6 +355,8 @@ class BytecodeGraphBuilder { const BytecodeAnalysis* bytecode_analysis_; Environment* environment_; BailoutId osr_ast_id_; + int currently_peeled_loop_offset_; + bool stack_check_; // Merge environments are snapshots of the environment at points where the // control flow merges. This models a forward data flow propagation of all diff --git a/src/compiler/js-inlining.cc b/src/compiler/js-inlining.cc index 4172998544..e55d8c0ada 100644 --- a/src/compiler/js-inlining.cc +++ b/src/compiler/js-inlining.cc @@ -551,8 +551,9 @@ Reduction JSInliner::ReduceJSCall(Node* node) { } BytecodeGraphBuilder graph_builder( parse_info.zone(), shared_info, feedback_vector, BailoutId::None(), - jsgraph(), call.frequency(), source_positions_, inlining_id, flags); - graph_builder.CreateGraph(false); + jsgraph(), call.frequency(), source_positions_, inlining_id, flags, + false); + graph_builder.CreateGraph(); // Extract the inlinee start/end nodes. start = graph()->start(); diff --git a/src/compiler/osr.cc b/src/compiler/osr.cc index 2de3df6354..969dabc6b4 100644 --- a/src/compiler/osr.cc +++ b/src/compiler/osr.cc @@ -255,8 +255,9 @@ void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common, } // namespace -void OsrHelper::Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common, - Zone* tmp_zone) { +void OsrHelper::Deconstruct(CompilationInfo* info, JSGraph* jsgraph, + CommonOperatorBuilder* common, Zone* tmp_zone) { + DCHECK(!info->is_optimizing_from_bytecode()); Graph* graph = jsgraph->graph(); Node* osr_normal_entry = nullptr; Node* osr_loop_entry = nullptr; diff --git a/src/compiler/osr.h b/src/compiler/osr.h index 075a9774a7..6cda358f71 100644 --- a/src/compiler/osr.h +++ b/src/compiler/osr.h @@ -6,6 +6,9 @@ #define V8_COMPILER_OSR_H_ #include "src/zone/zone.h" +// TODO(6409) This phase (and then the below explanations) are now only used +// when osring from the ast graph builder. When using Ignition bytecode, the OSR +// implementation is integrated directly to the graph building phase. // TurboFan structures OSR graphs in a way that separates almost all phases of // compilation from OSR implementation details. This is accomplished with @@ -95,8 +98,8 @@ class OsrHelper { // Deconstructs the artificial {OsrNormalEntry} and rewrites the graph so // that only the path corresponding to {OsrLoopEntry} remains. - void Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common, - Zone* tmp_zone); + void Deconstruct(CompilationInfo* info, JSGraph* jsgraph, + CommonOperatorBuilder* common, Zone* tmp_zone); // Prepares the frame w.r.t. OSR. void SetupFrame(Frame* frame); diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index a046830e79..ada793366e 100644 --- a/src/compiler/pipeline.cc +++ b/src/compiler/pipeline.cc @@ -1083,14 +1083,20 @@ struct OsrDeconstructionPhase { static const char* phase_name() { return "OSR deconstruction"; } void Run(PipelineData* data, Zone* temp_zone) { + // When the bytecode comes from Ignition, we do the OSR implementation + // during the graph building phase. + if (data->info()->is_optimizing_from_bytecode()) return; + GraphTrimmer trimmer(temp_zone, data->graph()); NodeVector roots(temp_zone); data->jsgraph()->GetCachedNodes(&roots); trimmer.TrimGraph(roots.begin(), roots.end()); - // TODO(neis): Use data->osr_helper() here once AST graph builder is gone. + // TODO(neis): Remove (the whole OsrDeconstructionPhase) when AST graph + // builder is gone. OsrHelper osr_helper(data->info()); - osr_helper.Deconstruct(data->jsgraph(), data->common(), temp_zone); + osr_helper.Deconstruct(data->info(), data->jsgraph(), data->common(), + temp_zone); } }; diff --git a/src/source-position-table.h b/src/source-position-table.h index c77c1ef26e..f185062694 100644 --- a/src/source-position-table.h +++ b/src/source-position-table.h @@ -61,6 +61,12 @@ class V8_EXPORT_PRIVATE SourcePositionTableBuilder { class V8_EXPORT_PRIVATE SourcePositionTableIterator { public: + // Used for saving/restoring the iterator. + struct IndexAndPosition { + int index_; + PositionTableEntry position_; + }; + // We expose two flavours of the iterator, depending on the argument passed // to the constructor: @@ -89,6 +95,13 @@ class V8_EXPORT_PRIVATE SourcePositionTableIterator { } bool done() const { return index_ == kDone; } + IndexAndPosition GetState() const { return {index_, current_}; } + + void RestoreState(const IndexAndPosition& saved_state) { + index_ = saved_state.index_; + current_ = saved_state.position_; + } + private: static const int kDone = -1; diff --git a/test/mjsunit/compiler/osr-try.js b/test/mjsunit/compiler/osr-try.js new file mode 100644 index 0000000000..26c30a886c --- /dev/null +++ b/test/mjsunit/compiler/osr-try.js @@ -0,0 +1,23 @@ +// 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 + +// This tests checks some possible wrong exception handling due to, +// for instance, the OSR loop peeling. If exception handlers are not updated +// correctly, when we run the second iteration of the outermost loop, which +// is the OSR optimised version, the try-catch will fail... which should not +// fail on a correct code. + +function toto() { + for (var a = 0; a < 2; a++) { + try { throw 'The exception should have been caught.'; } + catch(e) {} + for (var b = 0; b < 1; b++) { + %OptimizeOsr(); + } + } +} + +toto();