[turbofan] Fix bug in OSR deconstruction.

In constructing the transfer between loop copies, we need to merge the backedges from all the previous copies of the given loop. The control reduction will work out which ones are really reachable.

R=mstarzinger@chromium.org
BUG=

Review URL: https://codereview.chromium.org/1004993004

Cr-Commit-Position: refs/heads/master@{#27246}

src/compiler/opcodes.h

@@ -10,11 +10,11 @@
   V(Dead)                        \
   V(Loop)                        \
   V(Branch)                      \
+  V(Switch)                      \
   V(IfTrue)                      \
   V(IfFalse)                     \
   V(IfSuccess)                   \
   V(IfException)                 \
-  V(Switch)                      \
   V(IfValue)                     \
   V(IfDefault)                   \
   V(Merge)                       \
@@ -322,6 +322,10 @@ class IrOpcode {
     return value == kMerge || value == kLoop;
   }
 
+  static bool IsIfProjectionOpcode(Value value) {
+    return kIfTrue <= value && value <= kIfDefault;
+  }
+
   // Returns true if opcode for comparison operator.
   static bool IsComparisonOpcode(Value value) {
     return (kJSEqual <= value && value <= kJSGreaterThanOrEqual) ||

src/compiler/osr.cc

@@ -26,6 +26,18 @@ OsrHelper::OsrHelper(CompilationInfo* info)
                         info->osr_expr_stack_height()) {}
 
 
+#ifdef DEBUG
+#define TRACE_COND (FLAG_trace_turbo_graph && FLAG_trace_osr)
+#else
+#define TRACE_COND false
+#endif
+
+#define TRACE(...)                       \
+  do {                                   \
+    if (TRACE_COND) PrintF(__VA_ARGS__); \
+  } while (false)
+
+
 // Peel outer loops and rewire the graph so that control reduction can
 // produce a properly formed graph.
 static void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
@@ -44,6 +56,9 @@ static void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
     NodeVector* mapping =
         new (stuff) NodeVector(original_count, sentinel, tmp_zone);
     copies.push_back(mapping);
+    TRACE("OsrDuplication #%zu, depth %zu, header #%d:%s\n", copies.size(),
+          loop->depth(), loop_tree->HeaderNode(loop)->id(),
+          loop_tree->HeaderNode(loop)->op()->mnemonic());
 
     // Prepare the mapping for OSR values and the OSR loop entry.
     mapping->at(osr_normal_entry->id()) = dead;
@@ -54,6 +69,8 @@ static void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
          outer = outer->parent()) {
       for (Node* node : loop_tree->HeaderNodes(outer)) {
         mapping->at(node->id()) = dead;
+        TRACE(" ---- #%d:%s -> dead (header)\n", node->id(),
+              node->op()->mnemonic());
       }
     }
 
@@ -82,71 +99,132 @@ static void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
         NodeProperties::SetBounds(copy, NodeProperties::GetBounds(orig));
       }
       mapping->at(orig->id()) = copy;
+      TRACE(" copy #%d:%s -> #%d\n", orig->id(), orig->op()->mnemonic(),
+            copy->id());
     }
 
     // Fix missing inputs.
-    for (size_t i = 0; i < all.live.size(); i++) {
-      Node* orig = all.live[i];
+    for (Node* orig : all.live) {
       Node* copy = mapping->at(orig->id());
       for (int j = 0; j < copy->InputCount(); j++) {
-        Node* input = copy->InputAt(j);
-        if (input == sentinel)
+        if (copy->InputAt(j) == sentinel) {
           copy->ReplaceInput(j, mapping->at(orig->InputAt(j)->id()));
+        }
       }
     }
 
-    // Construct the transfer from the previous graph copies to the new copy.
+    // Construct the entry into this loop from previous copies.
+
+    // Gather the live loop header nodes, {loop_header} first.
     Node* loop_header = loop_tree->HeaderNode(loop);
-    NodeVector* previous =
-        copies.size() > 1 ? copies[copies.size() - 2] : nullptr;
-    const int backedges = loop_header->op()->ControlInputCount() - 1;
-    if (backedges == 1) {
-      // Simple case. Map the incoming edges to the loop to the previous copy.
-      for (Node* node : loop_tree->HeaderNodes(loop)) {
-        if (!all.IsLive(node)) continue;  // dead phi hanging off loop.
+    NodeVector header_nodes(tmp_zone);
+    header_nodes.reserve(loop->HeaderSize());
+    header_nodes.push_back(loop_header);  // put the loop header first.
+    for (Node* node : loop_tree->HeaderNodes(loop)) {
+      if (node != loop_header && all.IsLive(node)) {
+        header_nodes.push_back(node);
+      }
+    }
+
+    // Gather backedges from the previous copies of the inner loops of {loop}.
+    NodeVectorVector backedges(tmp_zone);
+    TRACE("Gathering backedges...\n");
+    for (int i = 1; i < loop_header->InputCount(); i++) {
+      if (TRACE_COND) {
+        Node* control = loop_header->InputAt(i);
+        size_t incoming_depth = 0;
+        for (int j = 0; j < control->op()->ControlInputCount(); j++) {
+          Node* k = NodeProperties::GetControlInput(control, j);
+          incoming_depth =
+              std::max(incoming_depth, loop_tree->ContainingLoop(k)->depth());
+        }
+
+        TRACE(" edge @%d #%d:%s, incoming depth %zu\n", i, control->id(),
+              control->op()->mnemonic(), incoming_depth);
+      }
+
+      for (int pos = static_cast<int>(copies.size()) - 1; pos >= 0; pos--) {
+        backedges.push_back(NodeVector(tmp_zone));
+        backedges.back().reserve(header_nodes.size());
+
+        NodeVector* previous_map = pos > 0 ? copies[pos - 1] : nullptr;
+
+        for (Node* node : header_nodes) {
+          Node* input = node->InputAt(i);
+          if (previous_map) input = previous_map->at(input->id());
+          backedges.back().push_back(input);
+          TRACE("   node #%d:%s(@%d) = #%d:%s\n", node->id(),
+                node->op()->mnemonic(), i, input->id(),
+                input->op()->mnemonic());
+        }
+      }
+    }
+
+    int backedge_count = static_cast<int>(backedges.size());
+    if (backedge_count == 1) {
+      // Simple case of single backedge, therefore a single entry.
+      int index = 0;
+      for (Node* node : header_nodes) {
         Node* copy = mapping->at(node->id());
-        Node* backedge = node->InputAt(1);
-        if (previous) backedge = previous->at(backedge->id());
-        copy->ReplaceInput(0, backedge);
+        Node* input = backedges[0][index];
+        copy->ReplaceInput(0, input);
+        TRACE(" header #%d:%s(0) => #%d:%s\n", copy->id(),
+              copy->op()->mnemonic(), input->id(), input->op()->mnemonic());
+        index++;
       }
     } else {
-      // Complex case. Multiple backedges. Introduce a merge for incoming edges.
-      tmp_inputs.clear();
-      for (int i = 0; i < backedges; i++) {
-        Node* backedge = loop_header->InputAt(i + 1);
-        if (previous) backedge = previous->at(backedge->id());
-        tmp_inputs.push_back(backedge);
-      }
-      Node* merge =
-          graph->NewNode(common->Merge(backedges), backedges, &tmp_inputs[0]);
-      for (Node* node : loop_tree->HeaderNodes(loop)) {
-        if (!all.IsLive(node)) continue;  // dead phi hanging off loop.
+      // Complex case of multiple backedges from previous copies requires
+      // merging the backedges to create the entry into the loop header.
+      Node* merge = nullptr;
+      int index = 0;
+      for (Node* node : header_nodes) {
+        // Gather edge inputs into {tmp_inputs}.
+        tmp_inputs.clear();
+        for (int edge = 0; edge < backedge_count; edge++) {
+          tmp_inputs.push_back(backedges[edge][index]);
+        }
         Node* copy = mapping->at(node->id());
+        Node* input;
         if (node == loop_header) {
-          // The entry to the loop is the merge.
+          // Create the merge for the entry into the loop header.
+          input = merge = graph->NewNode(common->Merge(backedge_count),
+                                         backedge_count, &tmp_inputs[0]);
           copy->ReplaceInput(0, merge);
         } else {
-          // Merge inputs to the phi at the loop entry.
-          tmp_inputs.clear();
-          for (int i = 0; i < backedges; i++) {
-            Node* backedge = node->InputAt(i + 1);
-            if (previous) backedge = previous->at(backedge->id());
-            tmp_inputs.push_back(backedge);
-          }
+          // Create a phi that merges values at entry into the loop header.
+          DCHECK_NOT_NULL(merge);
+          DCHECK(IrOpcode::IsPhiOpcode(node->opcode()));
           tmp_inputs.push_back(merge);
-          Node* phi =
-              graph->NewNode(common->ResizeMergeOrPhi(node->op(), backedges),
-                             backedges + 1, &tmp_inputs[0]);
+          Node* phi = input = graph->NewNode(
+              common->ResizeMergeOrPhi(node->op(), backedge_count),
+              backedge_count + 1, &tmp_inputs[0]);
           copy->ReplaceInput(0, phi);
         }
+
+        // Print the merge.
+        if (TRACE_COND) {
+          TRACE(" header #%d:%s(0) => #%d:%s(", copy->id(),
+                copy->op()->mnemonic(), input->id(), input->op()->mnemonic());
+          for (size_t i = 0; i < tmp_inputs.size(); i++) {
+            if (i > 0) TRACE(", ");
+            Node* input = tmp_inputs[i];
+            TRACE("#%d:%s", input->id(), input->op()->mnemonic());
+          }
+          TRACE(")\n");
+        }
+
+        index++;
       }
     }
   }
 
   // Kill the outer loops in the original graph.
+  TRACE("Killing outer loop headers...\n");
   for (LoopTree::Loop* outer = osr_loop->parent(); outer;
        outer = outer->parent()) {
-    loop_tree->HeaderNode(outer)->ReplaceUses(dead);
+    Node* loop_header = loop_tree->HeaderNode(outer);
+    loop_header->ReplaceUses(dead);
+    TRACE(" ---- #%d:%s\n", loop_header->id(), loop_header->op()->mnemonic());
   }
 
   // Merge the ends of the graph copies.

src/runtime/runtime-test.cc

@@ -111,10 +111,10 @@ RUNTIME_FUNCTION(Runtime_OptimizeFunctionOnNextCall) {
 
 RUNTIME_FUNCTION(Runtime_OptimizeOsr) {
   HandleScope scope(isolate);
-  RUNTIME_ASSERT(args.length() == 0);
+  RUNTIME_ASSERT(args.length() == 0 || args.length() == 1);
   Handle<JSFunction> function = Handle<JSFunction>::null();
 
-  {
+  if (args.length() == 0) {
     // Find the JavaScript function on the top of the stack.
     JavaScriptFrameIterator it(isolate);
     while (!it.done()) {
@@ -124,6 +124,10 @@ RUNTIME_FUNCTION(Runtime_OptimizeOsr) {
       }
     }
     if (function.is_null()) return isolate->heap()->undefined_value();
+  } else {
+    // Function was passed as an argument.
+    CONVERT_ARG_HANDLE_CHECKED(JSFunction, arg, 0);
+    function = arg;
   }
 
   // The following assertion was lifted from the DCHECK inside

src/runtime/runtime.h

@@ -63,7 +63,7 @@ namespace internal {
   F(RunningInSimulator, 0, 1)                  \
   F(IsConcurrentRecompilationSupported, 0, 1)  \
   F(OptimizeFunctionOnNextCall, -1, 1)         \
-  F(OptimizeOsr, 0, 1)                         \
+  F(OptimizeOsr, -1, 1)                        \
   F(NeverOptimizeFunction, 1, 1)               \
   F(GetOptimizationStatus, -1, 1)              \
   F(GetOptimizationCount, 1, 1)                \

test/cctest/compiler/test-osr.cc

@@ -36,6 +36,8 @@ static int CheckInputs(Node* node, Node* i0 = NULL, Node* i1 = NULL,
 
 static Operator kIntLt(IrOpcode::kInt32LessThan, Operator::kPure,
                        "Int32LessThan", 2, 0, 0, 1, 0, 0);
+static Operator kIntAdd(IrOpcode::kInt32Add, Operator::kPure, "Int32Add", 2, 0,
+                        0, 1, 0, 0);
 
 
 static const int kMaxOsrValues = 10;
@@ -489,3 +491,105 @@ TEST(Deconstruct_osr_nested2) {
   CheckInputs(new_outer_phi, new_entry_phi, new_inner_phi,
               T.jsgraph.ZeroConstant(), new_outer_loop);
 }
+
+
+Node* MakeCounter(JSGraph* jsgraph, Node* start, Node* loop) {
+  int count = loop->InputCount();
+  NodeVector tmp_inputs(jsgraph->graph()->zone());
+  for (int i = 0; i < count; i++) {
+    tmp_inputs.push_back(start);
+  }
+  tmp_inputs.push_back(loop);
+
+  Node* phi = jsgraph->graph()->NewNode(
+      jsgraph->common()->Phi(kMachInt32, count), count + 1, &tmp_inputs[0]);
+  Node* inc = jsgraph->graph()->NewNode(&kIntAdd, phi, jsgraph->OneConstant());
+
+  for (int i = 1; i < count; i++) {
+    phi->ReplaceInput(i, inc);
+  }
+  return phi;
+}
+
+
+TEST(Deconstruct_osr_nested3) {
+  OsrDeconstructorTester T(1);
+
+  // outermost loop.
+  While loop0(T, T.p0, false, 1);
+  Node* loop0_cntr = MakeCounter(&T.jsgraph, T.p0, loop0.loop);
+  loop0.branch->ReplaceInput(0, loop0_cntr);
+
+  // middle loop.
+  Node* loop1 = T.graph.NewNode(T.common.Loop(2), loop0.if_true, T.self);
+  loop1->ReplaceInput(0, loop0.if_true);
+  Node* loop1_phi =
+      T.graph.NewNode(T.common.Phi(kMachAnyTagged, 2), loop0_cntr, loop0_cntr);
+
+  // innermost (OSR) loop.
+  While loop2(T, T.p0, true, 1);
+  loop2.loop->ReplaceInput(0, loop1);
+
+  Node* loop2_cntr = MakeCounter(&T.jsgraph, loop1_phi, loop2.loop);
+  loop2_cntr->ReplaceInput(1, T.osr_values[0]);
+  Node* osr_phi = loop2_cntr;
+  Node* loop2_inc = loop2_cntr->InputAt(2);
+  loop2.branch->ReplaceInput(0, loop2_cntr);
+
+  loop1_phi->ReplaceInput(1, loop2_cntr);
+  loop0_cntr->ReplaceInput(1, loop2_cntr);
+
+  // Branch to either the outer or middle loop.
+  Node* branch = T.graph.NewNode(T.common.Branch(), loop2_cntr, loop2.exit);
+  Node* if_true = T.graph.NewNode(T.common.IfTrue(), branch);
+  Node* if_false = T.graph.NewNode(T.common.IfFalse(), branch);
+
+  loop0.loop->ReplaceInput(1, if_true);
+  loop1->ReplaceInput(1, if_false);
+
+  Node* ret =
+      T.graph.NewNode(T.common.Return(), loop0_cntr, T.start, loop0.exit);
+  Node* end = T.graph.NewNode(T.common.End(), ret);
+  T.graph.SetEnd(end);
+
+  T.DeconstructOsr();
+
+  // Check structure of deconstructed graph.
+  // Check loop2 (OSR loop) is directly connected to start.
+  CheckInputs(loop2.loop, T.start, loop2.if_true);
+  CheckInputs(osr_phi, T.osr_values[0], loop2_inc, loop2.loop);
+  CheckInputs(loop2.branch, osr_phi, loop2.loop);
+  CheckInputs(loop2.if_true, loop2.branch);
+  CheckInputs(loop2.exit, loop2.branch);
+  CheckInputs(branch, osr_phi, loop2.exit);
+  CheckInputs(if_true, branch);
+  CheckInputs(if_false, branch);
+
+  // Check structure of new_loop1.
+  Node* new_loop1_loop = FindSuccessor(if_false, IrOpcode::kLoop);
+  // TODO(titzer): check the internal copy of loop2.
+  USE(new_loop1_loop);
+
+  // Check structure of new_loop0.
+  Node* new_loop0_loop_entry = FindSuccessor(if_true, IrOpcode::kMerge);
+  Node* new_loop0_loop = FindSuccessor(new_loop0_loop_entry, IrOpcode::kLoop);
+  // TODO(titzer): check the internal copies of loop1 and loop2.
+
+  Node* new_loop0_branch = FindSuccessor(new_loop0_loop, IrOpcode::kBranch);
+  Node* new_loop0_if_true = FindSuccessor(new_loop0_branch, IrOpcode::kIfTrue);
+  Node* new_loop0_exit = FindSuccessor(new_loop0_branch, IrOpcode::kIfFalse);
+
+  USE(new_loop0_if_true);
+
+  Node* new_ret = T.graph.end()->InputAt(0);
+  CHECK_EQ(IrOpcode::kReturn, new_ret->opcode());
+
+  Node* new_loop0_phi = new_ret->InputAt(0);
+  CHECK_EQ(IrOpcode::kPhi, new_loop0_phi->opcode());
+  CHECK_EQ(new_loop0_loop, NodeProperties::GetControlInput(new_loop0_phi));
+  CHECK_EQ(new_loop0_phi, FindSuccessor(new_loop0_loop, IrOpcode::kPhi));
+
+  // Check that the return returns the phi from the OSR loop and control
+  // depends on the copy of the outer loop0.
+  CheckInputs(new_ret, new_loop0_phi, T.graph.start(), new_loop0_exit);
+}

test/mjsunit/compiler/osr-infinite.js

+// Copyright 2015 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: --use-osr --allow-natives-syntax --turbo-osr
+
+var global_counter = 0;
+
+function thrower() {
+  var x = global_counter++;
+  if (x == 5)  %OptimizeOsr(thrower.caller);
+  if (x == 10) throw "terminate";
+}
+
+%NeverOptimizeFunction(thrower);  // Don't want to inline the thrower.
+%NeverOptimizeFunction(test);     // Don't want to inline the func into test.
+
+function test(func) {
+  for (var i = 0; i < 3; i++) {
+    global_counter = 0;
+    assertThrows(func);
+  }
+}
+
+function n1() {
+  while (true) thrower();
+}
+
+function n2() {
+  while (true) while (true) thrower();
+}
+
+function n3() {
+  while (true) while (true) while (true) thrower();
+}
+
+function n4() {
+  while (true) while (true) while (true) while (true) thrower();
+}
+
+function b1(a) {
+  while (true) {
+    thrower();
+    if (a) break
+  }
+}
+
+
+function b2(a) {
+  while (true) {
+    while (true) {
+      thrower();
+      if (a) break
+    }
+  }
+}
+
+
+function b3(a) {
+  while (true) {
+    while (true) {
+      while (true) {
+        thrower();
+        if (a) break
+      }
+      if (a) break
+    }
+  }
+}
+
+
+test(n1);
+test(n2);
+test(n3);
+test(n4);
+test(b1);
+test(b2);
+test(b3);

test/mjsunit/compiler/osr-labeled.js

+// Copyright 2015 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 --use-osr --turbo-osr
+
+function foo() {
+  var sum = 0;
+  A: for (var i = 0; i < 5; i++) {
+    B: for (var j = 0; j < 5; j++) {
+      C: for (var k = 0; k < 10; k++) {
+        if (k === 5) %OptimizeOsr();
+        if (k === 6) break B;
+        sum++;
+      }
+    }
+  }
+  return sum;
+}
+
+assertEquals(30, foo());
+assertEquals(30, foo());
+
+function bar(a) {
+  var sum = 0;
+  A: for (var i = 0; i < 5; i++) {
+    B: for (var j = 0; j < 5; j++) {
+      C: for (var k = 0; k < 10; k++) {
+        sum++;
+        %OptimizeOsr();
+        if (a === 1) break A;
+        if (a === 2) break B;
+        if (a === 3) break C;
+      }
+    }
+  }
+  return sum;
+}
+
+assertEquals(1, bar(1));
+assertEquals(1, bar(1));
+
+assertEquals(5, bar(2));
+assertEquals(5, bar(2));
+
+assertEquals(25, bar(3));
+assertEquals(25, bar(3));