[turbofan] Simplify reduction if IfTrue and IfFalse and fix bugs.

R=mstarzinger@chromium.org
BUG=chromium:451958
LOG=Y

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

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

diff --git a/src/compiler/control-reducer.cc b/src/compiler/control-reducer.cc
index 0c26062..7f4bf31 100644 (file)
--- a/src/compiler/control-reducer.cc
+++ b/src/compiler/control-reducer.cc
@@ -357,8 +357,10 @@ class ControlReducerImpl {
 
     // Reduce branches, phis, and merges.
     switch (node->opcode()) {
-      case IrOpcode::kBranch:
-        return ReduceBranch(node);
+      case IrOpcode::kIfTrue:
+        return ReduceIfTrue(node);
+      case IrOpcode::kIfFalse:
+        return ReduceIfFalse(node);
       case IrOpcode::kLoop:
       case IrOpcode::kMerge:
         return ReduceMerge(node);
@@ -480,30 +482,31 @@ class ControlReducerImpl {
   }
 
   // Reduce branches if they have constant inputs.
-  Node* ReduceBranch(Node* node) {
-    Decision result = DecideCondition(node->InputAt(0));
-    if (result == kUnknown) return node;
-
-    TRACE(("BranchReduce: #%d:%s = %s\n", node->id(), node->op()->mnemonic(),
-           (result == kTrue) ? "true" : "false"));
-
-    // Replace IfTrue and IfFalse projections from this branch.
-    Node* control = NodeProperties::GetControlInput(node);
-    for (Edge edge : node->use_edges()) {
-      Node* use = edge.from();
-      if (use->opcode() == IrOpcode::kIfTrue) {
-        TRACE(("  IfTrue: #%d:%s\n", use->id(), use->op()->mnemonic()));
-        edge.UpdateTo(NULL);
-        ReplaceNode(use, (result == kTrue) ? control : dead());
-        control = NodeProperties::GetControlInput(node);  // Could change!
-      } else if (use->opcode() == IrOpcode::kIfFalse) {
-        TRACE(("  IfFalse: #%d:%s\n", use->id(), use->op()->mnemonic()));
-        edge.UpdateTo(NULL);
-        ReplaceNode(use, (result == kTrue) ? dead() : control);
-        control = NodeProperties::GetControlInput(node);  // Could change!
-      }
+  Node* ReduceIfTrue(Node* node) {
+    Node* branch = node->InputAt(0);
+    DCHECK_EQ(IrOpcode::kBranch, branch->opcode());
+    Decision result = DecideCondition(branch->InputAt(0));
+    if (result == kTrue) {
+      // fold a true branch by replacing IfTrue with the branch control.
+      TRACE(("BranchReduce: #%d:%s => #%d:%s\n", branch->id(),
+             branch->op()->mnemonic(), node->id(), node->op()->mnemonic()));
+      return branch->InputAt(1);
+    }
+    return result == kUnknown ? node : dead();
+  }
+
+  // Reduce branches if they have constant inputs.
+  Node* ReduceIfFalse(Node* node) {
+    Node* branch = node->InputAt(0);
+    DCHECK_EQ(IrOpcode::kBranch, branch->opcode());
+    Decision result = DecideCondition(branch->InputAt(0));
+    if (result == kFalse) {
+      // fold a false branch by replacing IfFalse with the branch control.
+      TRACE(("BranchReduce: #%d:%s => #%d:%s\n", branch->id(),
+             branch->op()->mnemonic(), node->id(), node->op()->mnemonic()));
+      return branch->InputAt(1);
     }
-    return control;
+    return result == kUnknown ? node : dead();
   }
 
   // Remove inputs to {node} corresponding to the dead inputs to {merge}
@@ -578,12 +581,19 @@ Node* ControlReducer::ReduceMergeForTesting(JSGraph* jsgraph,
 }
 
 
-Node* ControlReducer::ReduceBranchForTesting(JSGraph* jsgraph,
+Node* ControlReducer::ReduceIfNodeForTesting(JSGraph* jsgraph,
                                              CommonOperatorBuilder* common,
                                              Node* node) {
   Zone zone;
   ControlReducerImpl impl(&zone, jsgraph, common);
-  return impl.ReduceBranch(node);
+  switch (node->opcode()) {
+    case IrOpcode::kIfTrue:
+      return impl.ReduceIfTrue(node);
+    case IrOpcode::kIfFalse:
+      return impl.ReduceIfFalse(node);
+    default:
+      return node;
+  }
 }
 }
 }

diff --git a/src/compiler/control-reducer.h b/src/compiler/control-reducer.h
index e25bb88..ee5a097 100644 (file)
--- a/src/compiler/control-reducer.h
+++ b/src/compiler/control-reducer.h
@@ -25,7 +25,7 @@ class ControlReducer {
   // Testing interface.
   static Node* ReducePhiForTesting(JSGraph* graph,
                                    CommonOperatorBuilder* builder, Node* node);
-  static Node* ReduceBranchForTesting(JSGraph* graph,
+  static Node* ReduceIfNodeForTesting(JSGraph* graph,
                                       CommonOperatorBuilder* builder,
                                       Node* node);
   static Node* ReduceMergeForTesting(JSGraph* graph,

diff --git a/test/cctest/compiler/test-control-reducer.cc b/test/cctest/compiler/test-control-reducer.cc
index 13e60bc..c4a1190 100644 (file)
--- a/test/cctest/compiler/test-control-reducer.cc
+++ b/test/cctest/compiler/test-control-reducer.cc
@@ -17,6 +17,8 @@ using namespace v8::internal::compiler;
 
 static const size_t kNumLeafs = 4;
 
+enum Decision { kFalse, kUnknown, kTrue };
+
 // TODO(titzer): convert this whole file into unit tests.
 
 static int CheckInputs(Node* node, Node* i0 = NULL, Node* i1 = NULL,
@@ -175,10 +177,25 @@ class ControlReducerTester : HandleAndZoneScope {
     CheckInputs(end, expect);
   }
 
-  void ReduceBranch(Node* expect, Node* branch) {
-    Node* result =
-        ControlReducer::ReduceBranchForTesting(&jsgraph, &common, branch);
-    CHECK_EQ(expect, result);
+  void ReduceBranch(Decision expected, Node* branch) {
+    Node* control = branch->InputAt(1);
+    for (Node* use : branch->uses()) {
+      if (use->opcode() == IrOpcode::kIfTrue) {
+        Node* result =
+            ControlReducer::ReduceIfNodeForTesting(&jsgraph, &common, use);
+        if (expected == kTrue) CHECK_EQ(control, result);
+        if (expected == kFalse) CHECK_EQ(IrOpcode::kDead, result->opcode());
+        if (expected == kUnknown) CHECK_EQ(use, result);
+      } else if (use->opcode() == IrOpcode::kIfFalse) {
+        Node* result =
+            ControlReducer::ReduceIfNodeForTesting(&jsgraph, &common, use);
+        if (expected == kFalse) CHECK_EQ(control, result);
+        if (expected == kTrue) CHECK_EQ(IrOpcode::kDead, result->opcode());
+        if (expected == kUnknown) CHECK_EQ(use, result);
+      } else {
+        UNREACHABLE();
+      }
+    }
   }
 
   Node* Return(Node* val, Node* effect, Node* control) {
@@ -1028,7 +1045,7 @@ struct While {
 TEST(CBranchReduce_none1) {
   ControlReducerTester R;
   Diamond d(R, R.p0);
-  R.ReduceBranch(d.branch, d.branch);
+  R.ReduceBranch(kUnknown, d.branch);
 }
 
 
@@ -1037,7 +1054,7 @@ TEST(CBranchReduce_none2) {
   Diamond d1(R, R.p0);
   Diamond d2(R, R.p0);
   d2.chain(d1);
-  R.ReduceBranch(d2.branch, d2.branch);
+  R.ReduceBranch(kUnknown, d2.branch);
 }
 
 
@@ -1050,13 +1067,7 @@ TEST(CBranchReduce_true) {
 
   for (size_t i = 0; i < arraysize(true_values); i++) {
     Diamond d(R, true_values[i]);
-    Node* true_use = R.graph.NewNode(R.common.Merge(1), d.if_true);
-    Node* false_use = R.graph.NewNode(R.common.Merge(1), d.if_false);
-    R.ReduceBranch(R.start, d.branch);
-    CHECK_EQ(R.start, true_use->InputAt(0));
-    CHECK_EQ(IrOpcode::kDead, false_use->InputAt(0)->opcode());
-    CHECK(d.if_true->IsDead());   // replaced
-    CHECK(d.if_false->IsDead());  // replaced
+    R.ReduceBranch(kTrue, d.branch);
   }
 }
 
@@ -1068,13 +1079,7 @@ TEST(CBranchReduce_false) {
 
   for (size_t i = 0; i < arraysize(false_values); i++) {
     Diamond d(R, false_values[i]);
-    Node* true_use = R.graph.NewNode(R.common.Merge(1), d.if_true);
-    Node* false_use = R.graph.NewNode(R.common.Merge(1), d.if_false);
-    R.ReduceBranch(R.start, d.branch);
-    CHECK_EQ(R.start, false_use->InputAt(0));
-    CHECK_EQ(IrOpcode::kDead, true_use->InputAt(0)->opcode());
-    CHECK(d.if_true->IsDead());   // replaced
-    CHECK(d.if_false->IsDead());  // replaced
+    R.ReduceBranch(kFalse, d.branch);
   }
 }
 
@@ -1356,10 +1361,10 @@ TEST(CNonTermLoop_big2) {
   Branch b1(R, R.p0);
   Node* rt = R.graph.NewNode(R.common.Return(), R.one, R.start, b1.if_true);
 
-  Branch b2(R, R.zero, b1.if_false);
+  Node* loop = R.graph.NewNode(R.common.Loop(2), b1.if_false, R.start);
+  Branch b2(R, R.zero, loop);
+  loop->ReplaceInput(1, b2.if_false);
   Node* rf = R.graph.NewNode(R.common.Return(), R.zero, R.start, b2.if_true);
-  Node* loop = R.SetSelfReferences(
-      R.graph.NewNode(R.common.Loop(2), b2.if_false, R.self));
   Node* merge = R.graph.NewNode(R.common.Merge(2), rt, rf);
   R.end->ReplaceInput(0, merge);
 

diff --git a/test/mjsunit/regress/regress-451958.js b/test/mjsunit/regress/regress-451958.js
new file mode 100644 (file)
index 0000000..33695f2
--- /dev/null
+++ b/test/mjsunit/regress/regress-451958.js
@@ -0,0 +1,31 @@
+// Copyright 2015 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.
+
+// Flags: --allow-natives-syntax
+
+function k() { throw "e"; }
+var a = true;
+var a = false;
+function foo(a) {
+  var i, j;
+  if (a) {
+    for (i = 0; i < 1; j++) ;
+    for (i = 0; i < 1; k()) ;
+    for (i = 0; i < 1; i++) ;
+  }
+}
+%OptimizeFunctionOnNextCall(foo);
+foo();
+
+function bar() {
+var __v_45;
+  for (__v_45 = 0; __v_45 < 64; __v_63++) {
+  }
+  for (__v_45 = 0; __v_45 < 128; __v_36++) {
+  }
+  for (__v_45 = 128; __v_45 < 256; __v_45++) {
+  }
+}
+%OptimizeFunctionOnNextCall(bar);
+assertThrows(bar);