Commit 5d0a4327 authored by Ross McIlroy's avatar Ross McIlroy Committed by Commit Bot

Revert "[Turbofan] Merged the OSR phase into the graph building phase."

This reverts commit 69c8f16d.

Reason for revert: Causing crashes on Clusterfuzz - http://crbug.com/747154

BUG=chromium:747154

Original change's description:
> [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 <alexandret@google.com>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#46801}

TBR=rmcilroy@chromium.org,mstarzinger@chromium.org,leszeks@chromium.org,alexandret@google.com

Change-Id: Ifa9bf5d86e888a47cad7fb10446b36fda5029604
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6112, v8:6518
Reviewed-on: https://chromium-review.googlesource.com/581288Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46817}
parent 250ba28a
......@@ -80,9 +80,10 @@ 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; }
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; }
int osr_entry_point() const { return osr_entry_point_; }
// Gets the in-liveness for the bytecode at {offset}.
const BytecodeLivenessState* GetInLivenessFor(int offset) const;
......
This diff is collapsed.
......@@ -31,25 +31,16 @@ class BytecodeGraphBuilder {
JSGraph* jsgraph, CallFrequency invocation_frequency,
SourcePositionTable* source_positions,
int inlining_id = SourcePosition::kNotInlined,
JSTypeHintLowering::Flags flags = JSTypeHintLowering::kNoFlags,
bool stack_check = true);
JSTypeHintLowering::Flags flags = JSTypeHintLowering::kNoFlags);
// Creates a graph by visiting bytecodes.
void CreateGraph();
void CreateGraph(bool stack_check = true);
private:
class Environment;
class OsrIteratorState;
struct SubEnvironment;
void RemoveMergeEnvironmentsBeforeOffset(int limit_offset);
void AdvanceToOsrEntryAndPeelLoops(
interpreter::BytecodeArrayIterator* iterator,
SourcePositionTableIterator* source_position_iterator);
void VisitSingleBytecode(
SourcePositionTableIterator* source_position_iterator);
void VisitBytecodes();
void VisitBytecodes(bool stack_check);
// Get or create the node that represents the outer function closure.
Node* GetFunctionClosure();
......@@ -245,6 +236,10 @@ 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);
......@@ -256,7 +251,7 @@ class BytecodeGraphBuilder {
// Update the current position of the {SourcePositionTable} to that of the
// bytecode at {offset}, if any.
void UpdateSourcePosition(SourcePositionTableIterator* it, int offset);
void UpdateCurrentSourcePosition(SourcePositionTableIterator* it, int offset);
// Growth increment for the temporary buffer used to construct input lists to
// new nodes.
......@@ -304,7 +299,7 @@ class BytecodeGraphBuilder {
}
void set_bytecode_iterator(
interpreter::BytecodeArrayIterator* bytecode_iterator) {
const interpreter::BytecodeArrayIterator* bytecode_iterator) {
bytecode_iterator_ = bytecode_iterator;
}
......@@ -316,24 +311,6 @@ 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;
......@@ -355,8 +332,6 @@ 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
......
......@@ -552,9 +552,8 @@ 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,
false);
graph_builder.CreateGraph();
jsgraph(), call.frequency(), source_positions_, inlining_id, flags);
graph_builder.CreateGraph(false);
// Extract the inlinee start/end nodes.
start = graph()->start();
......
......@@ -255,9 +255,8 @@ void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
} // namespace
void OsrHelper::Deconstruct(CompilationInfo* info, JSGraph* jsgraph,
CommonOperatorBuilder* common, Zone* tmp_zone) {
DCHECK(!info->is_optimizing_from_bytecode());
void OsrHelper::Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common,
Zone* tmp_zone) {
Graph* graph = jsgraph->graph();
Node* osr_normal_entry = nullptr;
Node* osr_loop_entry = nullptr;
......
......@@ -6,9 +6,6 @@
#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
......@@ -98,8 +95,8 @@ class OsrHelper {
// Deconstructs the artificial {OsrNormalEntry} and rewrites the graph so
// that only the path corresponding to {OsrLoopEntry} remains.
void Deconstruct(CompilationInfo* info, JSGraph* jsgraph,
CommonOperatorBuilder* common, Zone* tmp_zone);
void Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common,
Zone* tmp_zone);
// Prepares the frame w.r.t. OSR.
void SetupFrame(Frame* frame);
......
......@@ -1085,20 +1085,14 @@ 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): Remove (the whole OsrDeconstructionPhase) when AST graph
// builder is gone.
// TODO(neis): Use data->osr_helper() here once AST graph builder is gone.
OsrHelper osr_helper(data->info());
osr_helper.Deconstruct(data->info(), data->jsgraph(), data->common(),
temp_zone);
osr_helper.Deconstruct(data->jsgraph(), data->common(), temp_zone);
}
};
......
......@@ -61,12 +61,6 @@ 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:
......@@ -95,13 +89,6 @@ 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;
......
// 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();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment