[turbofan] Avoid unnecessary copying of nodes during inlining.

Previously we first created a temporary graph for the inlinee and then
copied over all the nodes to the actual graph. This however introduces
unnecessary complexity, and we can instead just create the inlinee
inside the target graph.

R=jarin@chromium.org

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

src/compiler/ast-graph-builder.cc

@@ -416,7 +416,7 @@ class AstGraphBuilder::FrameStateBeforeAndAfter {
   FrameStateBeforeAndAfter(AstGraphBuilder* builder, BailoutId id_before)
       : builder_(builder), frame_state_before_(nullptr) {
     frame_state_before_ = id_before == BailoutId::None()
-                              ? builder_->jsgraph()->EmptyFrameState()
+                              ? builder_->GetEmptyFrameState()
                               : builder_->environment()->Checkpoint(id_before);
   }
 
@@ -435,7 +435,7 @@ class AstGraphBuilder::FrameStateBeforeAndAfter {
 
       Node* frame_state_after =
           id_after == BailoutId::None()
-              ? builder_->jsgraph()->EmptyFrameState()
+              ? builder_->GetEmptyFrameState()
               : builder_->environment()->Checkpoint(id_after, combine,
                                                     node_has_exception);
 
@@ -539,6 +539,18 @@ Node* AstGraphBuilder::GetNewTarget() {
   return new_target_.get();
 }
 
+Node* AstGraphBuilder::GetEmptyFrameState() {
+  if (!empty_frame_state_.is_set()) {
+    const Operator* op = common()->FrameState(
+        BailoutId::None(), OutputFrameStateCombine::Ignore(), nullptr);
+    Node* node = graph()->NewNode(
+        op, jsgraph()->EmptyStateValues(), jsgraph()->EmptyStateValues(),
+        jsgraph()->EmptyStateValues(), jsgraph()->NoContextConstant(),
+        jsgraph()->UndefinedConstant(), graph()->start());
+    empty_frame_state_.set(node);
+  }
+  return empty_frame_state_.get();
+}
 
 bool AstGraphBuilder::CreateGraph(bool stack_check) {
   Scope* scope = info()->scope();
@@ -875,7 +887,7 @@ Node* AstGraphBuilder::Environment::Checkpoint(BailoutId ast_id,
                                                OutputFrameStateCombine combine,
                                                bool owner_has_exception) {
   if (!builder()->info()->is_deoptimization_enabled()) {
-    return builder()->jsgraph()->EmptyFrameState();
+    return builder()->GetEmptyFrameState();
   }
 
   UpdateStateValues(&parameters_node_, 0, parameters_count());

src/compiler/ast-graph-builder.h

@@ -106,6 +106,9 @@ class AstGraphBuilder : public AstVisitor {
   // Optimization to cache loaded feedback vector.
   SetOncePointer<Node> feedback_vector_;
 
+  // Optimization to cache empty frame state.
+  SetOncePointer<Node> empty_frame_state_;
+
   // Control nodes that exit the function body.
   ZoneVector<Node*> exit_controls_;
 
@@ -167,6 +170,9 @@ class AstGraphBuilder : public AstVisitor {
   // Get or create the node that represents the incoming new target value.
   Node* GetNewTarget();
 
+  // Get or create the node that represents the empty frame state.
+  Node* GetEmptyFrameState();
+
   // Node creation helpers.
   Node* NewNode(const Operator* op, bool incomplete = false) {
     return MakeNode(op, 0, static_cast<Node**>(nullptr), incomplete);

src/compiler/graph.h

@@ -28,11 +28,30 @@ typedef uint32_t Mark;
 // out-of-line data associated with each node.
 typedef uint32_t NodeId;
 
-
-class Graph : public ZoneObject {
+class Graph final : public ZoneObject {
  public:
   explicit Graph(Zone* zone);
 
+  // Scope used when creating a subgraph for inlining. Automatically preserves
+  // the original start and end nodes of the graph, and resets them when you
+  // leave the scope.
+  class SubgraphScope final {
+   public:
+    explicit SubgraphScope(Graph* graph)
+        : graph_(graph), start_(graph->start()), end_(graph->end()) {}
+    ~SubgraphScope() {
+      graph_->SetStart(start_);
+      graph_->SetEnd(end_);
+    }
+
+   private:
+    Graph* const graph_;
+    Node* const start_;
+    Node* const end_;
+
+    DISALLOW_COPY_AND_ASSIGN(SubgraphScope);
+  };
+
   // Base implementation used by all factory methods.
   Node* NewNodeUnchecked(const Operator* op, int input_count,
                          Node* const* inputs, bool incomplete = false);

src/compiler/js-graph.cc

@@ -222,17 +222,19 @@ Node* JSGraph::ExternalConstant(Runtime::FunctionId function_id) {
 Node* JSGraph::EmptyFrameState() {
   Node* empty_frame_state = cached_nodes_[kEmptyFrameState];
   if (!empty_frame_state || empty_frame_state->IsDead()) {
-    Node* state_values = graph()->NewNode(common()->StateValues(0));
     empty_frame_state = graph()->NewNode(
         common()->FrameState(BailoutId::None(),
                              OutputFrameStateCombine::Ignore(), nullptr),
-        state_values, state_values, state_values, NoContextConstant(),
-        UndefinedConstant(), graph()->start());
+        EmptyStateValues(), EmptyStateValues(), EmptyStateValues(),
+        NoContextConstant(), UndefinedConstant(), graph()->start());
     cached_nodes_[kEmptyFrameState] = empty_frame_state;
   }
   return empty_frame_state;
 }
 
+Node* JSGraph::EmptyStateValues() {
+  return CACHED(kEmptyStateValues, graph()->NewNode(common()->StateValues(0)));
+}
 
 Node* JSGraph::Dead() {
   return CACHED(kDead, graph()->NewNode(common()->Dead()));

src/compiler/js-graph.h

@@ -127,6 +127,10 @@ class JSGraph : public ZoneObject {
   // cannot deopt.
   Node* EmptyFrameState();
 
+  // Creates an empty StateValues node, used when we don't have any concrete
+  // values for a certain part of the frame state.
+  Node* EmptyStateValues();
+
   // Create a control node that serves as dependency for dead nodes.
   Node* Dead();
 
@@ -159,6 +163,7 @@ class JSGraph : public ZoneObject {
     kOneConstant,
     kNaNConstant,
     kEmptyFrameState,
+    kEmptyStateValues,
     kDead,
     kNumCachedNodes  // Must remain last.
   };

src/compiler/js-inlining.cc

@@ -8,7 +8,6 @@
 #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"
@@ -77,63 +76,6 @@ class JSCallAccessor {
 };
 
 
-class CopyVisitor {
- public:
-  CopyVisitor(Graph* source_graph, Graph* target_graph, Zone* temp_zone)
-      : sentinel_op_(IrOpcode::kDead, Operator::kNoProperties, "Sentinel", 0, 0,
-                     0, 0, 0, 0),
-        sentinel_(target_graph->NewNode(&sentinel_op_)),
-        copies_(source_graph->NodeCount(), sentinel_, temp_zone),
-        source_graph_(source_graph),
-        target_graph_(target_graph),
-        temp_zone_(temp_zone) {}
-
-  Node* GetCopy(Node* orig) { return copies_[orig->id()]; }
-
-  void CopyGraph() {
-    NodeVector inputs(temp_zone_);
-    // TODO(bmeurer): AllNodes should be turned into something like
-    // Graph::CollectNodesReachableFromEnd() and the gray set stuff should be
-    // removed since it's only needed by the visualizer.
-    AllNodes all(temp_zone_, source_graph_);
-    // Copy all nodes reachable from end.
-    for (Node* orig : all.live) {
-      Node* copy = GetCopy(orig);
-      if (copy != sentinel_) {
-        // Mapping already exists.
-        continue;
-      }
-      // Copy the node.
-      inputs.clear();
-      for (Node* input : orig->inputs()) inputs.push_back(copies_[input->id()]);
-      copy = target_graph_->NewNode(orig->op(), orig->InputCount(),
-                                    inputs.empty() ? nullptr : &inputs[0]);
-      copies_[orig->id()] = copy;
-    }
-    // For missing inputs.
-    for (Node* orig : all.live) {
-      Node* copy = copies_[orig->id()];
-      for (int i = 0; i < copy->InputCount(); ++i) {
-        Node* input = copy->InputAt(i);
-        if (input == sentinel_) {
-          copy->ReplaceInput(i, GetCopy(orig->InputAt(i)));
-        }
-      }
-    }
-  }
-
-  const NodeVector& copies() const { return copies_; }
-
- private:
-  Operator const sentinel_op_;
-  Node* const sentinel_;
-  NodeVector copies_;
-  Graph* const source_graph_;
-  Graph* const target_graph_;
-  Zone* const temp_zone_;
-};
-
-
 Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
                                 Node* frame_state, Node* start, Node* end) {
   // The scheduler is smart enough to place our code; we just ensure {control}
@@ -448,69 +390,71 @@ Reduction JSInliner::ReduceJSCall(Node* node, Handle<JSFunction> function) {
         shared_info->DebugName()->ToCString().get(),
         info_->shared_info()->DebugName()->ToCString().get());
 
-  // Run the loop assignment analyzer on the inlinee.
-  AstLoopAssignmentAnalyzer loop_assignment_analyzer(&zone, &info);
-  LoopAssignmentAnalysis* loop_assignment = loop_assignment_analyzer.Analyze();
-
-  // Run the type hint analyzer on the inlinee.
-  TypeHintAnalyzer type_hint_analyzer(&zone);
-  TypeHintAnalysis* type_hint_analysis =
-      type_hint_analyzer.Analyze(handle(shared_info->code(), info.isolate()));
-
-  // TODO(mstarzinger): We could use the temporary zone for the graph because
-  // nodes are copied. This however leads to Zone-Types being allocated in the
-  // wrong zone and makes the engine explode at high speeds. Explosion bad!
-  Graph graph(jsgraph_->zone());
-  JSGraph jsgraph(info.isolate(), &graph, jsgraph_->common(),
-                  jsgraph_->javascript(), jsgraph_->simplified(),
-                  jsgraph_->machine());
-  AstGraphBuilder graph_builder(local_zone_, &info, &jsgraph, loop_assignment,
-                                type_hint_analysis);
-  graph_builder.CreateGraph(false);
-
-  CopyVisitor visitor(&graph, jsgraph_->graph(), &zone);
-  visitor.CopyGraph();
-
-  Node* start = visitor.GetCopy(graph.start());
-  Node* end = visitor.GetCopy(graph.end());
+  // Create the subgraph for the inlinee.
+  Node* start;
+  Node* end;
+  {
+    // Run the loop assignment analyzer on the inlinee.
+    AstLoopAssignmentAnalyzer loop_assignment_analyzer(&zone, &info);
+    LoopAssignmentAnalysis* loop_assignment =
+        loop_assignment_analyzer.Analyze();
+
+    // Run the type hint analyzer on the inlinee.
+    TypeHintAnalyzer type_hint_analyzer(&zone);
+    TypeHintAnalysis* type_hint_analysis =
+        type_hint_analyzer.Analyze(handle(shared_info->code(), info.isolate()));
+
+    // Run the AstGraphBuilder to create the subgraph.
+    Graph::SubgraphScope scope(graph());
+    AstGraphBuilder graph_builder(&zone, &info, jsgraph(), loop_assignment,
+                                  type_hint_analysis);
+    graph_builder.CreateGraph(false);
+
+    // Extract the inlinee start/end nodes.
+    start = graph()->start();
+    end = graph()->end();
+  }
+
   Node* frame_state = call.frame_state_after();
   Node* new_target = jsgraph_->UndefinedConstant();
 
-  // Insert nodes around the call that model the behavior required for a
-  // constructor dispatch (allocate implicit receiver and check return value).
-  // This models the behavior usually accomplished by our {JSConstructStub}.
-  // Note that the context has to be the callers context (input to call node).
-  Node* receiver = jsgraph_->UndefinedConstant();  // Implicit receiver.
-  if (node->opcode() == IrOpcode::kJSCallConstruct &&
-      NeedsImplicitReceiver(shared_info)) {
-    Node* effect = NodeProperties::GetEffectInput(node);
-    Node* context = NodeProperties::GetContextInput(node);
-    Node* create = jsgraph_->graph()->NewNode(
-        jsgraph_->javascript()->Create(), call.target(), call.new_target(),
-        context, call.frame_state_before(), effect);
-    NodeProperties::ReplaceEffectInput(node, create);
-    // Insert a check of the return value to determine whether the return value
-    // or the implicit receiver should be selected as a result of the call.
-    Node* check = jsgraph_->graph()->NewNode(
-        jsgraph_->javascript()->CallRuntime(Runtime::kInlineIsJSReceiver, 1),
-        node, context, node, start);
-    Node* select = jsgraph_->graph()->NewNode(
-        jsgraph_->common()->Select(MachineRepresentation::kTagged), check, node,
-        create);
-    NodeProperties::ReplaceUses(node, select, check, node, node);
-    NodeProperties::ReplaceValueInput(select, node, 1);
-    NodeProperties::ReplaceValueInput(check, node, 0);
-    NodeProperties::ReplaceEffectInput(check, node);
-    receiver = create;  // The implicit receiver.
-  }
-
-  // Swizzle the inputs of the {JSCallConstruct} node to look like inputs to a
-  // normal {JSCallFunction} node so that the rest of the inlining machinery
-  // behaves as if we were dealing with a regular function invocation.
+  // Inline {JSCallConstruct} requires some additional magic.
   if (node->opcode() == IrOpcode::kJSCallConstruct) {
+    // Insert nodes around the call that model the behavior required for a
+    // constructor dispatch (allocate implicit receiver and check return value).
+    // This models the behavior usually accomplished by our {JSConstructStub}.
+    // Note that the context has to be the callers context (input to call node).
+    Node* receiver = jsgraph_->UndefinedConstant();  // Implicit receiver.
+    if (NeedsImplicitReceiver(shared_info)) {
+      Node* effect = NodeProperties::GetEffectInput(node);
+      Node* context = NodeProperties::GetContextInput(node);
+      Node* create = jsgraph_->graph()->NewNode(
+          jsgraph_->javascript()->Create(), call.target(), call.new_target(),
+          context, call.frame_state_before(), effect);
+      NodeProperties::ReplaceEffectInput(node, create);
+      // Insert a check of the return value to determine whether the return
+      // value
+      // or the implicit receiver should be selected as a result of the call.
+      Node* check = jsgraph_->graph()->NewNode(
+          jsgraph_->javascript()->CallRuntime(Runtime::kInlineIsJSReceiver, 1),
+          node, context, node, start);
+      Node* select = jsgraph_->graph()->NewNode(
+          jsgraph_->common()->Select(MachineRepresentation::kTagged), check,
+          node, create);
+      NodeProperties::ReplaceUses(node, select, check, node, node);
+      NodeProperties::ReplaceValueInput(select, node, 1);
+      NodeProperties::ReplaceValueInput(check, node, 0);
+      NodeProperties::ReplaceEffectInput(check, node);
+      receiver = create;  // The implicit receiver.
+    }
+
+    // Swizzle the inputs of the {JSCallConstruct} node to look like inputs to a
+    // normal {JSCallFunction} node so that the rest of the inlining machinery
+    // behaves as if we were dealing with a regular function invocation.
     new_target = call.new_target();  // Retrieve new target value input.
     node->RemoveInput(call.formal_arguments() + 1);  // Drop new target.
     node->InsertInput(jsgraph_->graph()->zone(), 1, receiver);
+
     // Insert a construct stub frame into the chain of frame states. This will
     // reconstruct the proper frame when deoptimizing within the constructor.
     frame_state = CreateArtificialFrameState(
@@ -570,6 +514,8 @@ Reduction JSInliner::ReduceJSCall(Node* node, Handle<JSFunction> function) {
   return InlineCall(node, new_target, context, frame_state, start, end);
 }
 
+Graph* JSInliner::graph() const { return jsgraph()->graph(); }
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/js-inlining.h

@@ -36,9 +36,12 @@ class JSInliner final : public AdvancedReducer {
   Reduction ReduceJSCall(Node* node, Handle<JSFunction> function);
 
  private:
-  Zone* local_zone_;
+  Graph* graph() const;
+  JSGraph* jsgraph() const { return jsgraph_; }
+
+  Zone* const local_zone_;
   CompilationInfo* info_;
-  JSGraph* jsgraph_;
+  JSGraph* const jsgraph_;
 
   Node* CreateArtificialFrameState(Node* node, Node* outer_frame_state,
                                    int parameter_count,

src/compiler/pipeline.cc

@@ -804,7 +804,9 @@ struct InliningPhase {
     AddReducer(data, &graph_reducer, &native_context_specialization);
     AddReducer(data, &graph_reducer, &context_specialization);
     AddReducer(data, &graph_reducer, &call_reducer);
-    AddReducer(data, &graph_reducer, &inlining);
+    if (!data->info()->is_optimizing_from_bytecode()) {
+      AddReducer(data, &graph_reducer, &inlining);
+    }
     graph_reducer.ReduceGraph();
   }
 };

test/mjsunit/compiler/inline-dead-jscreate.js

+// Copyright 2016 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.
+
+var bar = 0;
+
+function baz() { return this; }
+
+function foo() {
+  bar += 1;
+  if (bar === 2) throw new baz();
+}
+
+foo();