[turbofan] Also inline into try blocks.

This removes test/webkit/fast/js/stack-overflow-arrity-catch.js, which tests that the stack overflows in a very particular way. It doesn't seem to test anything important, and only used to work because we didn't inline into try-blocks.

BUG=
R=jarin

Review-Url: https://codereview.chromium.org/2216353002
Cr-Commit-Position: refs/heads/master@{#38976}

src/compiler/all-nodes.cc

@@ -14,13 +14,26 @@ AllNodes::AllNodes(Zone* local_zone, const Graph* graph, bool only_inputs)
     : reachable(local_zone),
       is_reachable_(graph->NodeCount(), false, local_zone),
       only_inputs_(only_inputs) {
-  Node* end = graph->end();
+  Mark(local_zone, graph->end(), graph);
+}
+
+AllNodes::AllNodes(Zone* local_zone, Node* end, const Graph* graph,
+                   bool only_inputs)
+    : reachable(local_zone),
+      is_reachable_(graph->NodeCount(), false, local_zone),
+      only_inputs_(only_inputs) {
+  Mark(local_zone, end, graph);
+}
+
+void AllNodes::Mark(Zone* local_zone, Node* end, const Graph* graph) {
+  DCHECK_LT(end->id(), graph->NodeCount());
   is_reachable_[end->id()] = true;
   reachable.push_back(end);
-  // Find all nodes reachable from end.
+  // Find all nodes reachable from {end}.
   for (size_t i = 0; i < reachable.size(); i++) {
-    for (Node* input : reachable[i]->inputs()) {
-      if (input == nullptr || input->id() >= graph->NodeCount()) {
+    for (Node* const input : reachable[i]->inputs()) {
+      if (input == nullptr) {
+        // TODO(titzer): print a warning.
         continue;
       }
       if (!is_reachable_[input->id()]) {
@@ -28,7 +41,7 @@ AllNodes::AllNodes(Zone* local_zone, const Graph* graph, bool only_inputs)
         reachable.push_back(input);
       }
     }
-    if (!only_inputs) {
+    if (!only_inputs_) {
       for (Node* use : reachable[i]->uses()) {
         if (use == nullptr || use->id() >= graph->NodeCount()) {
           continue;

src/compiler/all-nodes.h

@@ -16,9 +16,13 @@ namespace compiler {
 // from end.
 class AllNodes {
  public:
-  // Constructor. Traverses the graph and builds the {reachable} sets. When
-  // {only_inputs} is true, find the nodes reachable through input edges;
-  // these are all live nodes.
+  // Constructor. Traverses the graph and builds the {reachable} set of nodes
+  // reachable from {end}. When {only_inputs} is true, find the nodes
+  // reachable through input edges; these are all live nodes.
+  AllNodes(Zone* local_zone, Node* end, const Graph* graph,
+           bool only_inputs = true);
+  // Constructor. Traverses the graph and builds the {reachable} set of nodes
+  // reachable from the End node.
   AllNodes(Zone* local_zone, const Graph* graph, bool only_inputs = true);
 
   bool IsLive(Node* node) {
@@ -35,6 +39,8 @@ class AllNodes {
   NodeVector reachable;  // Nodes reachable from end.
 
  private:
+  void Mark(Zone* local_zone, Node* end, const Graph* graph);
+
   BoolVector is_reachable_;
   const bool only_inputs_;
 };

src/compiler/js-inlining-heuristic.cc

@@ -65,7 +65,7 @@ Reduction JSInliningHeuristic::Reduce(Node* node) {
   if (info_->shared_info()->asm_function()) return NoChange();
   if (function->shared()->asm_function()) return NoChange();
 
-  // Stop inlinining once the maximum allowed level is reached.
+  // Stop inlining once the maximum allowed level is reached.
   int level = 0;
   for (Node* frame_state = NodeProperties::GetFrameStateInput(node);
        frame_state->opcode() == IrOpcode::kFrameState;

src/compiler/js-inlining.cc

@@ -8,6 +8,7 @@
 #include "src/ast/ast.h"
 #include "src/ast/scopes.h"
 #include "src/compiler.h"
+#include "src/compiler/all-nodes.h"
 #include "src/compiler/ast-graph-builder.h"
 #include "src/compiler/ast-loop-assignment-analyzer.h"
 #include "src/compiler/common-operator.h"
@@ -72,9 +73,10 @@ class JSCallAccessor {
   Node* call_;
 };
 
-
 Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
-                                Node* frame_state, Node* start, Node* end) {
+                                Node* frame_state, Node* start, Node* end,
+                                Node* exception_target,
+                                const NodeVector& uncaught_subcalls) {
   // The scheduler is smart enough to place our code; we just ensure {control}
   // becomes the control input of the start of the inlinee, and {effect} becomes
   // the effect input of the start of the inlinee.
@@ -131,6 +133,44 @@ Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
     }
   }
 
+  if (exception_target != nullptr) {
+    // Link uncaught calls in the inlinee to {exception_target}
+    int subcall_count = static_cast<int>(uncaught_subcalls.size());
+    if (subcall_count > 0) {
+      TRACE(
+          "Inlinee contains %d calls without IfException; "
+          "linking to existing IfException\n",
+          subcall_count);
+    }
+    NodeVector on_exception_nodes(local_zone_);
+    for (Node* subcall : uncaught_subcalls) {
+      Node* on_exception =
+          graph()->NewNode(common()->IfException(), subcall, subcall);
+      on_exception_nodes.push_back(on_exception);
+    }
+
+    DCHECK_EQ(subcall_count, static_cast<int>(on_exception_nodes.size()));
+    if (subcall_count > 0) {
+      Node* control_output =
+          graph()->NewNode(common()->Merge(subcall_count), subcall_count,
+                           &on_exception_nodes.front());
+      NodeVector values_effects(local_zone_);
+      values_effects = on_exception_nodes;
+      values_effects.push_back(control_output);
+      Node* value_output = graph()->NewNode(
+          common()->Phi(MachineRepresentation::kTagged, subcall_count),
+          subcall_count + 1, &values_effects.front());
+      Node* effect_output =
+          graph()->NewNode(common()->EffectPhi(subcall_count),
+                           subcall_count + 1, &values_effects.front());
+      ReplaceWithValue(exception_target, value_output, effect_output,
+                       control_output);
+    } else {
+      ReplaceWithValue(exception_target, exception_target, exception_target,
+                       jsgraph()->Dead());
+    }
+  }
+
   NodeVector values(local_zone_);
   NodeVector effects(local_zone_);
   NodeVector controls(local_zone_);
@@ -270,7 +310,6 @@ Reduction JSInliner::Reduce(Node* node) {
   return ReduceJSCall(node, function);
 }
 
-
 Reduction JSInliner::ReduceJSCall(Node* node, Handle<JSFunction> function) {
   DCHECK(IrOpcode::IsInlineeOpcode(node->opcode()));
   JSCallAccessor call(node);
@@ -344,12 +383,35 @@ Reduction JSInliner::ReduceJSCall(Node* node, Handle<JSFunction> function) {
     }
   }
 
-  // TODO(turbofan): Inlining into a try-block is not yet supported.
-  if (NodeProperties::IsExceptionalCall(node)) {
-    TRACE("Not inlining %s into %s because of surrounding try-block\n",
+  // Find the IfException node, if any.
+  Node* exception_target = nullptr;
+  for (Edge edge : node->use_edges()) {
+    if (NodeProperties::IsControlEdge(edge) &&
+        edge.from()->opcode() == IrOpcode::kIfException) {
+      DCHECK_NULL(exception_target);
+      exception_target = edge.from();
+    }
+  }
+
+  NodeVector uncaught_subcalls(local_zone_);
+
+  if (exception_target != nullptr) {
+    if (!FLAG_inline_into_try) {
+      TRACE(
+          "Try block surrounds #%d:%s and --no-inline-into-try active, so not "
+          "inlining %s into %s.\n",
+          exception_target->id(), exception_target->op()->mnemonic(),
           shared_info->DebugName()->ToCString().get(),
           info_->shared_info()->DebugName()->ToCString().get());
-    return NoChange();
+      return NoChange();
+    } else {
+      TRACE(
+          "Inlining %s into %s regardless of surrounding try-block to catcher "
+          "#%d:%s\n",
+          shared_info->DebugName()->ToCString().get(),
+          info_->shared_info()->DebugName()->ToCString().get(),
+          exception_target->id(), exception_target->op()->mnemonic());
+    }
   }
 
   Zone zone(info_->isolate()->allocator());
@@ -388,7 +450,7 @@ Reduction JSInliner::ReduceJSCall(Node* node, Handle<JSFunction> function) {
         shared_info->DebugName()->ToCString().get(),
         info_->shared_info()->DebugName()->ToCString().get());
 
-  // If function was lazily compiled, it's literals array may not yet be set up.
+  // If function was lazily compiled, its literals array may not yet be set up.
   JSFunction::EnsureLiterals(function);
 
   // Create the subgraph for the inlinee.
@@ -416,6 +478,29 @@ Reduction JSInliner::ReduceJSCall(Node* node, Handle<JSFunction> function) {
     end = graph()->end();
   }
 
+  if (exception_target != nullptr) {
+    // Find all uncaught 'calls' in the inlinee.
+    AllNodes inlined_nodes(local_zone_, end, graph());
+    for (Node* subnode : inlined_nodes.reachable) {
+      // Every possibly throwing node with an IfSuccess should get an
+      // IfException.
+      if (subnode->op()->HasProperty(Operator::kNoThrow)) {
+        continue;
+      }
+      bool hasIfException = false;
+      for (Node* use : subnode->uses()) {
+        if (use->opcode() == IrOpcode::kIfException) {
+          hasIfException = true;
+          break;
+        }
+      }
+      if (!hasIfException) {
+        DCHECK_EQ(2, subnode->op()->ControlOutputCount());
+        uncaught_subcalls.push_back(subnode);
+      }
+    }
+  }
+
   Node* frame_state = call.frame_state();
   Node* new_target = jsgraph()->UndefinedConstant();
 
@@ -512,7 +597,8 @@ Reduction JSInliner::ReduceJSCall(Node* node, Handle<JSFunction> function) {
         FrameStateType::kArgumentsAdaptor, shared_info);
   }
 
-  return InlineCall(node, new_target, context, frame_state, start, end);
+  return InlineCall(node, new_target, context, frame_state, start, end,
+                    exception_target, uncaught_subcalls);
 }
 
 Graph* JSInliner::graph() const { return jsgraph()->graph(); }

src/compiler/js-inlining.h

@@ -54,7 +54,9 @@ class JSInliner final : public AdvancedReducer {
   Node* CreateTailCallerFrameState(Node* node, Node* outer_frame_state);
 
   Reduction InlineCall(Node* call, Node* new_target, Node* context,
-                       Node* frame_state, Node* start, Node* end);
+                       Node* frame_state, Node* start, Node* end,
+                       Node* exception_target,
+                       const NodeVector& uncaught_subcalls);
 };
 
 }  // namespace compiler

src/flag-definitions.h

@@ -405,6 +405,8 @@ DEFINE_BOOL(flush_optimized_code_cache, false,
 DEFINE_BOOL(inline_construct, true, "inline constructor calls")
 DEFINE_BOOL(inline_arguments, true, "inline functions with arguments object")
 DEFINE_BOOL(inline_accessors, true, "inline JavaScript accessors")
+DEFINE_BOOL(inline_into_try, false, "inline into try blocks")
+DEFINE_IMPLICATION(turbo, inline_into_try)
 DEFINE_INT(escape_analysis_iterations, 2,
            "maximum number of escape analysis fix-point iterations")
 

test/cctest/test-heap-profiler.cc

@@ -2557,8 +2557,33 @@ TEST(ArrayGrowLeftTrim) {
   heap_profiler->StopTrackingHeapObjects();
 }
 
+TEST(TrackHeapAllocationsWithInlining) {
+  v8::HandleScope scope(v8::Isolate::GetCurrent());
+  LocalContext env;
+
+  v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
+  heap_profiler->StartTrackingHeapObjects(true);
+
+  CompileRun(record_trace_tree_source);
+
+  AllocationTracker* tracker =
+      reinterpret_cast<i::HeapProfiler*>(heap_profiler)->allocation_tracker();
+  CHECK(tracker);
+  // Resolve all function locations.
+  tracker->PrepareForSerialization();
+  // Print for better diagnostics in case of failure.
+  tracker->trace_tree()->Print(tracker);
+
+  const char* names[] = {"", "start", "f_0_0"};
+  AllocationTraceNode* node = FindNode(tracker, ArrayVector(names));
+  CHECK(node);
+  CHECK_GE(node->allocation_count(), 12u);
+  CHECK_GE(node->allocation_size(), 4 * node->allocation_count());
+  heap_profiler->StopTrackingHeapObjects();
+}
 
-TEST(TrackHeapAllocations) {
+TEST(TrackHeapAllocationsWithoutInlining) {
+  i::FLAG_max_inlined_source_size = 0;  // Disable inlining
   v8::HandleScope scope(v8::Isolate::GetCurrent());
   LocalContext env;
 

test/webkit/fast/js/stack-overflow-arrity-catch.js → test/mjsunit/stack-overflow-arity-catch-noinline.js

@@ -21,63 +21,67 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 // SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-description('Test that if an arrity check causes a stack overflow, the exception goes to the right catch');
+// Flags: --allow-natives-syntax
+
+var stackOverflowIn20ArgFn = false, gotRegexCatch = false, gotDateCatch = false;
 
 function funcWith20Args(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8,
                         arg9, arg10, arg11, arg12, arg13, arg14, arg15,
                         arg16, arg17, arg18, arg19, arg20)
 {
-    debug("ERROR: Shouldn't arrive in 20 arg function!");
+    assertUnreachable("shouldn't arrive in non-inlined 20 arg function after stack overflow");
 }
 
-var gotRightCatch = false, gotWrongCatch1 = false, gotWrongCatch2 = false;
+// If we should run with --turbo, then make sure {funcWith20Args} does
+// not get inlined.
+%NeverOptimizeFunction(funcWith20Args);
 
-function test1()
+function mutual_recursion_1()
 {
     try {
-        test2();
+        mutual_recursion_2();
     } catch (err) {
         // Should get here because of stack overflow,
-        // now cause a stack overflow exception due to arrity processing
+        // now cause a stack overflow exception due to arity processing
         try {
             var dummy = new RegExp('a|b|c');
         } catch(err) {
-            // (1) It is dendent on the stack size if we arrive here, in (2) or
+            // (1) It is dependent on the stack size if we arrive here, in (2) or
             // both.
-            gotWrongCatch1 = true;
+            gotRegexCatch = true;
         }
 
         try {
             funcWith20Args(1, 2, 3);
         } catch (err2) {
-            gotRightCatch = true;
+            stackOverflowIn20ArgFn = true;
         }
     }
 }
 
-function test2()
+function mutual_recursion_2()
 {
     try {
         var dummy = new Date();
     } catch(err) {
-        // (2) It is dendent on the stack size if we arrive here, in (1) or
+        // (2) It is dependent on the stack size if we arrive here, in (1) or
         // both.
-        gotWrongCatch2 = true;
+        gotDateCatch = true;
     }
 
     try {
-        test1();
+        mutual_recursion_1();
     } catch (err) {
         // Should get here because of stack overflow,
-        // now cause a stack overflow exception due to arrity processing
+        // now cause a stack overflow exception due to arity processing
         try {
             funcWith20Args(1, 2, 3, 4, 5, 6);
         } catch (err2) {
-            gotRightCatch = true;
+            stackOverflowIn20ArgFn = true;
         }
     }
 }
 
-test1();
+mutual_recursion_1();
 
-shouldBeTrue("gotRightCatch");
+assertTrue(stackOverflowIn20ArgFn);

test/webkit/fast/js/stack-overflow-arrity-catch-expected.txt

-# Copyright 2013 the V8 project authors. All rights reserved.
-# Copyright (C) 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
-#
-# Redistribution and use in source and binary forms, with or without
-# modification, are permitted provided that the following conditions
-# are met:
-# 1.  Redistributions of source code must retain the above copyright
-#     notice, this list of conditions and the following disclaimer.
-# 2.  Redistributions in binary form must reproduce the above copyright
-#     notice, this list of conditions and the following disclaimer in the
-#     documentation and/or other materials provided with the distribution.
-#
-# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY
-# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY
-# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
-# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
-# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
-# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
-# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-Test that if an arrity check causes a stack overflow, the exception goes to the right catch
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-
-PASS gotRightCatch is true
-PASS successfullyParsed is true
-
-TEST COMPLETE
-