[turbofan] Add AdvancedReducer::ReplaceWithValue() method and convert JSInlining...

Note that this is just a duplication for now. We'll want to get rid of the
NodeProperties::ReplaceWithValue() method in the long run.

R=bmeurer@chromium.org
BUG=

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

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

diff --git a/src/compiler/control-reducer.cc b/src/compiler/control-reducer.cc
index 6910e6c..59c63f7 100644 (file)
--- a/src/compiler/control-reducer.cc
+++ b/src/compiler/control-reducer.cc
@@ -554,6 +554,8 @@ class DummyEditor final : public AdvancedReducer::Editor {
     node->ReplaceUses(replacement);
   }
   void Revisit(Node* node) final {}
+  void ReplaceWithValue(Node* node, Node* value, Node* effect,
+                        Node* control) final {}
 };
 
 }  // namespace

diff --git a/src/compiler/graph-reducer.cc b/src/compiler/graph-reducer.cc
index fbd97bf..8718815 100644 (file)
--- a/src/compiler/graph-reducer.cc
+++ b/src/compiler/graph-reducer.cc
@@ -8,6 +8,7 @@
 #include "src/compiler/graph.h"
 #include "src/compiler/graph-reducer.h"
 #include "src/compiler/node.h"
+#include "src/compiler/node-properties.h"
 
 namespace v8 {
 namespace internal {
@@ -209,6 +210,40 @@ void GraphReducer::Replace(Node* node, Node* replacement, NodeId max_id) {
 }
 
 
+void GraphReducer::ReplaceWithValue(Node* node, Node* value, Node* effect,
+                                    Node* control) {
+  if (!effect && node->op()->EffectInputCount() > 0) {
+    effect = NodeProperties::GetEffectInput(node);
+  }
+  if (control == nullptr && node->op()->ControlInputCount() > 0) {
+    control = NodeProperties::GetControlInput(node);
+  }
+
+  // Requires distinguishing between value, effect and control edges.
+  for (Edge edge : node->use_edges()) {
+    Node* user = edge.from();
+    if (NodeProperties::IsControlEdge(edge)) {
+      if (user->opcode() == IrOpcode::kIfSuccess) {
+        Replace(user, control);
+      } else if (user->opcode() == IrOpcode::kIfException) {
+        // TODO(titzer): replace with dead control from JSGraph, and
+        // require the control reducer to propagate it.
+        UNREACHABLE();
+      } else {
+        UNREACHABLE();
+      }
+    } else if (NodeProperties::IsEffectEdge(edge)) {
+      DCHECK_NOT_NULL(effect);
+      edge.UpdateTo(effect);
+      Revisit(user);
+    } else {
+      edge.UpdateTo(value);
+      Revisit(user);
+    }
+  }
+}
+
+
 void GraphReducer::Pop() {
   Node* node = stack_.top().node;
   state_.Set(node, State::kVisited);

diff --git a/src/compiler/graph-reducer.h b/src/compiler/graph-reducer.h
index 1c90b30..aee2bca 100644 (file)
--- a/src/compiler/graph-reducer.h
+++ b/src/compiler/graph-reducer.h
@@ -74,6 +74,12 @@ class AdvancedReducer : public Reducer {
     virtual void Replace(Node* node, Node* replacement) = 0;
     // Revisit the {node} again later.
     virtual void Revisit(Node* node) = 0;
+    // Replace value uses of {node} with {value} and effect uses of {node} with
+    // {effect}. If {effect == NULL}, then use the effect input to {node}. All
+    // control uses will be relaxed assuming {node} cannot throw.
+    virtual void ReplaceWithValue(Node* node, Node* value,
+                                  Node* effect = nullptr,
+                                  Node* control = nullptr) = 0;
   };
 
   explicit AdvancedReducer(Editor* editor) : editor_(editor) {}
@@ -91,6 +97,11 @@ class AdvancedReducer : public Reducer {
     DCHECK_NOT_NULL(editor_);
     editor_->Revisit(node);
   }
+  void ReplaceWithValue(Node* node, Node* value, Node* effect = nullptr,
+                        Node* control = nullptr) {
+    DCHECK_NOT_NULL(editor_);
+    editor_->ReplaceWithValue(node, value, effect, control);
+  }
 
  private:
   Editor* const editor_;
@@ -126,6 +137,13 @@ class GraphReducer final : public AdvancedReducer::Editor {
 
   // Replace {node} with {replacement}.
   void Replace(Node* node, Node* replacement) final;
+
+  // Replace value uses of {node} with {value} and effect uses of {node} with
+  // {effect}. If {effect == NULL}, then use the effect input to {node}. All
+  // control uses will be relaxed assuming {node} cannot throw.
+  void ReplaceWithValue(Node* node, Node* value, Node* effect = nullptr,
+                        Node* control = nullptr) final;
+
   // Replace all uses of {node} with {replacement} if the id of {replacement} is
   // less than or equal to {max_id}. Otherwise, replace all uses of {node} whose
   // id is less than or equal to {max_id} with the {replacement}.

diff --git a/src/compiler/js-inlining.cc b/src/compiler/js-inlining.cc
index 6d8824f..af47cb8 100644 (file)
--- a/src/compiler/js-inlining.cc
+++ b/src/compiler/js-inlining.cc
@@ -59,13 +59,10 @@ class JSCallFunctionAccessor {
 };
 
 
-namespace {
-
 // A facade on a JSFunction's graph to facilitate inlining. It assumes
 // that the function graph has only one return statement, and provides
 // {UnifyReturn} to convert a function graph to that end.
-class Inlinee {
- public:
+struct Inlinee {
   Inlinee(Node* start, Node* end) : start_(start), end_(end) {}
 
   // Returns the last regular control node, that is
@@ -103,14 +100,9 @@ class Inlinee {
     return total_parameters() - 3;
   }
 
-  // Inline this graph at {call}, use {jsgraph} and its zone to create
-  // any new nodes.
-  Reduction InlineAtCall(JSGraph* jsgraph, Node* call);
-
   // Ensure that only a single return reaches the end node.
   static void UnifyReturn(JSGraph* jsgraph);
 
- private:
   Node* start_;
   Node* end_;
 };
@@ -218,7 +210,7 @@ class CopyVisitor {
 };
 
 
-Reduction Inlinee::InlineAtCall(JSGraph* jsgraph, Node* call) {
+Reduction JSInliner::InlineCall(Node* call, Inlinee& inlinee) {
   // 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.
@@ -226,12 +218,12 @@ Reduction Inlinee::InlineAtCall(JSGraph* jsgraph, Node* call) {
   Node* effect = NodeProperties::GetEffectInput(call);
 
   // Context is last argument.
-  int inlinee_context_index = static_cast<int>(total_parameters()) - 1;
+  int inlinee_context_index = static_cast<int>(inlinee.total_parameters()) - 1;
   // {inliner_inputs} counts JSFunction, Receiver, arguments, but not
   // context, effect, control.
   int inliner_inputs = call->op()->ValueInputCount();
   // Iterate over all uses of the start node.
-  for (Edge edge : start_->use_edges()) {
+  for (Edge edge : inlinee.start_->use_edges()) {
     Node* use = edge.from();
     switch (use->opcode()) {
       case IrOpcode::kParameter: {
@@ -239,14 +231,14 @@ Reduction Inlinee::InlineAtCall(JSGraph* jsgraph, Node* call) {
         if (index < inliner_inputs && index < inlinee_context_index) {
           // There is an input from the call, and the index is a value
           // projection but not the context, so rewire the input.
-          NodeProperties::ReplaceWithValue(use, call->InputAt(index));
+          ReplaceWithValue(use, call->InputAt(index));
         } else if (index == inlinee_context_index) {
           // TODO(turbofan): We always context specialize inlinees currently, so
           // we should never get here.
           UNREACHABLE();
         } else if (index < inlinee_context_index) {
           // Call has fewer arguments than required, fill with undefined.
-          NodeProperties::ReplaceWithValue(use, jsgraph->UndefinedConstant());
+          ReplaceWithValue(use, jsgraph_->UndefinedConstant());
         } else {
           // We got too many arguments, discard for now.
           // TODO(sigurds): Fix to treat arguments array correctly.
@@ -265,14 +257,12 @@ Reduction Inlinee::InlineAtCall(JSGraph* jsgraph, Node* call) {
     }
   }
 
-  NodeProperties::ReplaceWithValue(call, value_output(), effect_output(),
-                                   control_output());
+  ReplaceWithValue(call, inlinee.value_output(), inlinee.effect_output(),
+                   inlinee.control_output());
 
-  return Reducer::Replace(value_output());
+  return Replace(inlinee.value_output());
 }
 
-}  // namespace
-
 
 void JSInliner::AddClosureToFrameState(Node* frame_state,
                                        Handle<JSFunction> jsfunction) {
@@ -381,7 +371,7 @@ Reduction JSInliner::Reduce(Node* node) {
     }
   }
 
-  return inlinee.InlineAtCall(jsgraph_, node);
+  return InlineCall(node, inlinee);
 }
 
 }  // namespace compiler

diff --git a/src/compiler/js-inlining.h b/src/compiler/js-inlining.h
index 174b1e9..2f8dfa0 100644 (file)
--- a/src/compiler/js-inlining.h
+++ b/src/compiler/js-inlining.h
@@ -13,14 +13,19 @@ namespace internal {
 namespace compiler {
 
 class JSCallFunctionAccessor;
+struct Inlinee;
 
-class JSInliner final : public Reducer {
+class JSInliner final : public AdvancedReducer {
  public:
   enum Mode { kBuiltinsInlining, kGeneralInlining };
 
-  JSInliner(Mode mode, Zone* local_zone, CompilationInfo* info,
+  JSInliner(Editor* editor, Mode mode, Zone* local_zone, CompilationInfo* info,
             JSGraph* jsgraph)
-      : mode_(mode), local_zone_(local_zone), info_(info), jsgraph_(jsgraph) {}
+      : AdvancedReducer(editor),
+        mode_(mode),
+        local_zone_(local_zone),
+        info_(info),
+        jsgraph_(jsgraph) {}
 
   Reduction Reduce(Node* node) final;
 
@@ -34,6 +39,8 @@ class JSInliner final : public Reducer {
                                          Handle<JSFunction> jsfunction,
                                          Zone* temp_zone);
   void AddClosureToFrameState(Node* frame_state, Handle<JSFunction> jsfunction);
+
+  Reduction InlineCall(Node* call, Inlinee& inlinee);
 };
 
 }  // namespace compiler

diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc
index 197f100..7760018 100644 (file)
--- a/src/compiler/pipeline.cc
+++ b/src/compiler/pipeline.cc
@@ -504,11 +504,11 @@ struct InliningPhase {
   void Run(PipelineData* data, Zone* temp_zone) {
     SourcePositionTable::Scope pos(data->source_positions(),
                                    SourcePosition::Unknown());
-    JSInliner inliner(data->info()->is_inlining_enabled()
-                          ? JSInliner::kGeneralInlining
-                          : JSInliner::kBuiltinsInlining,
-                      temp_zone, data->info(), data->jsgraph());
     GraphReducer graph_reducer(data->graph(), temp_zone);
+    JSInliner inliner(&graph_reducer, data->info()->is_inlining_enabled()
+                                          ? JSInliner::kGeneralInlining
+                                          : JSInliner::kBuiltinsInlining,
+                      temp_zone, data->info(), data->jsgraph());
     AddReducer(data, &graph_reducer, &inliner);
     graph_reducer.ReduceGraph();
   }

diff --git a/test/unittests/compiler/graph-reducer-unittest.h b/test/unittests/compiler/graph-reducer-unittest.h
index 4cde964..2b0651d 100644 (file)
--- a/test/unittests/compiler/graph-reducer-unittest.h
+++ b/test/unittests/compiler/graph-reducer-unittest.h
@@ -15,6 +15,7 @@ namespace compiler {
 struct MockAdvancedReducerEditor : public AdvancedReducer::Editor {
   MOCK_METHOD1(Revisit, void(Node*));
   MOCK_METHOD2(Replace, void(Node*, Node*));
+  MOCK_METHOD4(ReplaceWithValue, void(Node*, Node*, Node*, Node*));
 };
 
 }  // namespace compiler