[Turbofan] Merged the OSR phase into the graph building phase (reland)

Reland of https://chromium-review.googlesource.com/c/543042/.

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: Ia02f2138f54fc79cab2f02fed68d9bb522d6ce14
Reviewed-on: https://chromium-review.googlesource.com/584756
Commit-Queue: Alexandre Talon <alexandret@google.com>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46899}

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;
 

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);
@@ -247,11 +252,11 @@ class BytecodeGraphBuilder {
   void BuildLoopExitsUntilLoop(int loop_offset);
 
   // Simulates entry and exit of exception handlers.
-  void EnterAndExitExceptionHandlers(int current_offset);
+  void ExitThenEnterExceptionHandlers(int current_offset);
 
   // 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

src/compiler/js-inlining.cc

@@ -552,8 +552,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();

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;

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);

src/compiler/pipeline.cc

@@ -1085,14 +1085,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);
   }
 };
 

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;
 

test/mjsunit/compiler/osr-try-catch.js

+// 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 SingleLoop() {
+  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();
+    }
+  }
+}
+
+
+// These function could also fail if the exception handlers are not updated at
+// the right time: a JSStackCheck gets created for the print, just after the
+// bytecode for the while LoopHeader. If the OSR phase did not exit properly
+// the exception before visiting the bytecode for the print, it will fail
+// because some IfSuccess gets created for nothing (the IfException will
+// become dead code and removed).
+function EmptyBody() {
+  try {; } catch(e) {; }
+  var a = 0;
+  while (1) {
+    %OptimizeOsr();
+    print("foo");
+
+    if (a == 1) break;
+    a++;
+  }
+}
+
+function NestedLoops() {
+  for (var a = 0; a < 2; a++) {
+    try {; } catch(e) {; }
+    %OptimizeOsr();
+    var b = 0;
+    while (1) {
+      print("bar");
+
+      if (b == 1) break;
+      b++;
+    }
+  }
+}
+
+
+SingleLoop();
+EmptyBody();
+NestedLoops();