[turbofan] Properly delete nodes in the GraphReducer.

This is required to enable a value numbering reducer, which needs to be
able to recognize "dead nodes".

TEST=compiler-unittests,cctest
R=jarin@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23675 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/compiler/compiler.gyp

@@ -21,6 +21,7 @@
         'change-lowering-unittest.cc',
         'common-operator-unittest.cc',
         'compiler-test-utils.h',
+        'graph-reducer-unittest.cc',
         'graph-unittest.cc',
         'graph-unittest.h',
         'instruction-selector-unittest.cc',

src/compiler/graph-reducer-unittest.cc

+// Copyright 2014 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.
+
+#include "src/compiler/graph.h"
+#include "src/compiler/graph-reducer.h"
+#include "src/compiler/operator.h"
+#include "src/test/test-utils.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+using testing::_;
+using testing::DefaultValue;
+using testing::Return;
+using testing::Sequence;
+using testing::StrictMock;
+
+namespace v8 {
+namespace internal {
+namespace compiler {
+
+namespace {
+
+SimpleOperator OP0(0, Operator::kNoWrite, 0, 0, "op0");
+
+
+struct MockReducer : public Reducer {
+  MOCK_METHOD1(Reduce, Reduction(Node*));
+};
+
+}  // namespace
+
+
+class GraphReducerTest : public TestWithZone {
+ public:
+  GraphReducerTest() : graph_(zone()) {}
+
+  static void SetUpTestCase() {
+    TestWithZone::SetUpTestCase();
+    DefaultValue<Reduction>::Set(Reducer::NoChange());
+  }
+
+  static void TearDownTestCase() {
+    DefaultValue<Reduction>::Clear();
+    TestWithZone::TearDownTestCase();
+  }
+
+ protected:
+  void ReduceNode(Node* node, Reducer* r) {
+    GraphReducer reducer(graph());
+    reducer.AddReducer(r);
+    reducer.ReduceNode(node);
+  }
+
+  void ReduceNode(Node* node, Reducer* r1, Reducer* r2) {
+    GraphReducer reducer(graph());
+    reducer.AddReducer(r1);
+    reducer.AddReducer(r2);
+    reducer.ReduceNode(node);
+  }
+
+  void ReduceNode(Node* node, Reducer* r1, Reducer* r2, Reducer* r3) {
+    GraphReducer reducer(graph());
+    reducer.AddReducer(r1);
+    reducer.AddReducer(r2);
+    reducer.AddReducer(r3);
+    reducer.ReduceNode(node);
+  }
+
+  Graph* graph() { return &graph_; }
+
+ private:
+  Graph graph_;
+};
+
+
+TEST_F(GraphReducerTest, ReduceOnceForEveryReducer) {
+  StrictMock<MockReducer> r1, r2;
+  Node* node0 = graph()->NewNode(&OP0);
+  EXPECT_CALL(r1, Reduce(node0));
+  EXPECT_CALL(r2, Reduce(node0));
+  ReduceNode(node0, &r1, &r2);
+}
+
+
+TEST_F(GraphReducerTest, ReduceAgainAfterChanged) {
+  Sequence s1, s2;
+  StrictMock<MockReducer> r1, r2, r3;
+  Node* node0 = graph()->NewNode(&OP0);
+  EXPECT_CALL(r1, Reduce(node0));
+  EXPECT_CALL(r2, Reduce(node0));
+  EXPECT_CALL(r3, Reduce(node0)).InSequence(s1, s2).WillOnce(
+      Return(Reducer::Changed(node0)));
+  EXPECT_CALL(r1, Reduce(node0)).InSequence(s1);
+  EXPECT_CALL(r2, Reduce(node0)).InSequence(s2);
+  ReduceNode(node0, &r1, &r2, &r3);
+}
+
+
+TEST_F(GraphReducerTest, OperatorIsNullAfterReplace) {
+  StrictMock<MockReducer> r;
+  Node* node0 = graph()->NewNode(&OP0);
+  Node* node1 = graph()->NewNode(&OP0);
+  EXPECT_CALL(r, Reduce(node0)).WillOnce(Return(Reducer::Replace(node1)));
+  ReduceNode(node0, &r);
+  EXPECT_EQ(NULL, node0->op());
+  EXPECT_EQ(&OP0, node1->op());
+}
+
+}  // namespace compiler
+}  // namespace internal
+}  // namespace v8

src/compiler/graph-reducer.cc

@@ -49,8 +49,8 @@ void GraphReducer::ReduceNode(Node* node) {
         // If {node} was replaced by an old node, unlink {node} and assume that
         // {replacement} was already reduced and finish.
         if (replacement->id() < before) {
-          node->RemoveAllInputs();
           node->ReplaceUses(replacement);
+          node->Kill();
           return;
         }
         // Otherwise, {node} was replaced by a new node. Replace all old uses of
@@ -59,7 +59,9 @@ void GraphReducer::ReduceNode(Node* node) {
         node->ReplaceUsesIf(
             std::bind2nd(std::ptr_fun(&NodeIdIsLessThan), before), replacement);
         // Unlink {node} if it's no longer used.
-        if (node->uses().empty()) node->RemoveAllInputs();
+        if (node->uses().empty()) {
+          node->Kill();
+        }
         // Rerun all the reductions on the {replacement}.
         skip = reducers_.end();
         node = replacement;
@@ -90,6 +92,7 @@ void GraphReducer::ReduceGraph() {
 
 
 // TODO(titzer): partial graph reductions.
-}
-}
-}  // namespace v8::internal::compiler
+
+}  // namespace compiler
+}  // namespace internal
+}  // namespace v8

src/compiler/node.cc

@@ -10,6 +10,14 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+void Node::Kill() {
+  DCHECK_NOT_NULL(op());
+  RemoveAllInputs();
+  DCHECK(uses().empty());
+  set_op(NULL);
+}
+
+
 void Node::CollectProjections(NodeVector* projections) {
   for (size_t i = 0; i < projections->size(); i++) {
     (*projections)[i] = NULL;

src/compiler/node.h

@@ -53,6 +53,7 @@ class Node : public GenericNode<NodeData, Node> {
       : GenericNode<NodeData, Node>(graph, input_count) {}
 
   void Initialize(Operator* op) { set_op(op); }
+  void Kill();
 
   void CollectProjections(ZoneVector<Node*>* projections);
   Node* FindProjection(int32_t projection_index);

test/cctest/compiler/test-graph-reducer.cc

@@ -470,9 +470,9 @@ TEST(ReduceForward1) {
     reducer.ReduceGraph();
     CHECK_EQ(before, graph.NodeCount());
     CHECK_EQ(&OPB0, n1->op());
-    CHECK_EQ(&OPB1, n2->op());
+    CHECK_EQ(NULL, n2->op());
     CHECK_EQ(n1, end->InputAt(0));
-    CHECK_EQ(&OPB1, n3->op());
+    CHECK_EQ(NULL, n3->op());
     CHECK_EQ(n1, end->InputAt(0));
     CHECK_EQ(&OPB2, end->op());
     CHECK_EQ(0, n2->UseCount());