[turbofan] Also inline into try blocks.

src/compiler/js-inlining.cc

Index: src/compiler/js-inlining.cc
diff --git a/src/compiler/js-inlining.cc b/src/compiler/js-inlining.cc
index 7aa82c91cb1909c9a5d26a81503dc4699eb86401..c8eced7a1960f281df32f55e13eed26462258d55 100644
--- a/src/compiler/js-inlining.cc
+++ b/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"
@@ -71,9 +72,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,
+                                NodeVector uncaught_subcalls) {

[Jarin, 2016/08/10 12:02:52]

You should pass the uncought_subcalls as a pointer or (better) as a const
reference. Passing by value will make an unnecessary copy.

[bgeron, 2016/08/10 14:37:26]

Agree, done.

   // 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.
@@ -130,6 +132,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 (uncaught_subcalls.size() > 0) {

[Jarin, 2016/08/10 12:02:52]

Nit: why don't you use subcall_count that you defined just above?

[bgeron, 2016/08/10 14:37:26]

Done.

+      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_);
@@ -269,7 +309,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);
@@ -343,12 +382,25 @@ 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",
-          shared_info->DebugName()->ToCString().get(),
-          info_->shared_info()->DebugName()->ToCString().get());
-    return NoChange();
+  // 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) {
+    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());
@@ -415,6 +467,31 @@ 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.live) {
+      // Every possibly throwing node with an IfSuccess should get an
+      // IfException.
+      if (subnode->op()->HasProperty(Operator::kNoThrow)) {
+        continue;
+      }
+      bool hasIfSuccess = false;
+      bool hasIfException = false;
+      for (Node* use : subnode->uses()) {
+        if (use->opcode() == IrOpcode::kIfSuccess) {

[Jarin, 2016/08/10 12:02:52]

I assume this won't be needed once every no-no-throw node has an ifsuccess
continuation. Right?

[bgeron, 2016/08/10 14:37:26]

Hm, I fixed some things to make them saner, but currently some no-no-throw nodes
still lack an IfSuccess continuation, specifically the Throw runtime call (or
something like that) has only an IfException. I'm verifying that the all control
uses of no-no-throw nodes are either IfSuccess or IfException. 

Speaking of which, I just realize that we don't check that no-no-throw nodes
which are reachable from End have any control uses at all. Should we check that?

[Jarin, 2016/08/11 09:54:23]

Actually, if Throw does not have IfSuccess, this condition would not be right if
you inline a throw into try-catch.

I would be more comfortable if we did not have to care about presence of
IfSuccess here.

[bgeron, 2016/08/11 18:55:55]

Removed that condition. 

To make this work, I had to depend on an earlier-landed patch which adds
kNoThrow to some opcodes. This screws up the delta between patch sets, should
you happen to use that.  

Also, I had to disable one assertion in a test: apparently suddenly inlining
works slightly differently now. I don't completely understand how, and I'm not
sure if I care enough.

+          hasIfSuccess = true;
+        } else if (use->opcode() == IrOpcode::kIfException) {
+          hasIfException = true;
+        }
+      }
+      if (hasIfSuccess && !hasIfException) {
+        DCHECK_EQ(2, subnode->op()->ControlOutputCount());
+        uncaught_subcalls.push_back(subnode);
+      }
+    }
+  }
+
   Node* frame_state = call.frame_state();
   Node* new_target = jsgraph()->UndefinedConstant();
 
@@ -513,7 +590,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(); }