[turbofan] Remove unused diamonds during control reduction.

src/compiler/control-reducer.cc

Index: src/compiler/control-reducer.cc
diff --git a/src/compiler/control-reducer.cc b/src/compiler/control-reducer.cc
index 136064faa7424e7d04b1a3593299392258a0c9b4..df84d525a4e4da8ec39f939ccd995a49064e048b 100644
--- a/src/compiler/control-reducer.cc
+++ b/src/compiler/control-reducer.cc
@@ -507,8 +507,6 @@ class ControlReducerImpl {
       index++;
     }
 
-    if (live > 1 && live == node->InputCount()) return node;  // nothing to do.
-
     TRACE(("ReduceMerge: #%d:%s (%d live)\n", node->id(),
            node->op()->mnemonic(), live));
 
@@ -527,15 +525,46 @@ class ControlReducerImpl {
       return node->InputAt(live_index);
     }
 
-    // Edit phis in place, removing dead inputs and revisiting them.
-    for (Node* const phi : phis) {
-      TRACE(("  PhiInMerge: #%d:%s (%d live)\n", phi->id(),
-             phi->op()->mnemonic(), live));
-      RemoveDeadInputs(node, phi);
-      Revisit(phi);
+    DCHECK_LE(2, live);
+
+    if (live < node->InputCount()) {
+      // Edit phis in place, removing dead inputs and revisiting them.
+      for (Node* const phi : phis) {
+        TRACE(("  PhiInMerge: #%d:%s (%d live)\n", phi->id(),
+               phi->op()->mnemonic(), live));
+        RemoveDeadInputs(node, phi);
+        Revisit(phi);
+      }
+      // Edit the merge in place, removing dead inputs.
+      RemoveDeadInputs(node, node);
+    }
+
+    DCHECK_EQ(live, node->InputCount());
+
+    // Check if it's an unused diamond.
+    if (live == 2 && phis.empty()) {
+      Node* node0 = node->InputAt(0);
+      Node* node1 = node->InputAt(1);
+      if (((node0->opcode() == IrOpcode::kIfTrue &&
+            node1->opcode() == IrOpcode::kIfFalse) ||
+           (node0->opcode() == IrOpcode::kIfTrue &&
+            node1->opcode() == IrOpcode::kIfFalse)) &&

[brucedawson, 2015/03/13 18:38:21]

This test looks wrong. This showed up as a new VC++ /analyze warning:

src\v8\src\objects.cc(2128) : warning C6237: (<zero> && <expression>) is always
zero.  <expression> is never evaluated and might have side effects.

I'm not sure what the  correct check is -- presumably either testing for both
being kIfTrue or both being kIfFalse, or else checking for node0/1 being
kIfTrue/kIfFalse or node0/1 being kIfFalse/kIfTrue. Or maybe something else.

[Jarin, 2015/03/14 19:17:34]

Good catch. This should test that node0/1 is kIfTrue/kIfFalse or
kIfFalse/kIfTrue.

+          node0->OwnedBy(node) && node1->OwnedBy(node)) {
+        Node* branch0 = NodeProperties::GetControlInput(node0);
+        Node* branch1 = NodeProperties::GetControlInput(node1);
+        if (branch0 == branch1) {
+          // It's a dead diamond, i.e. neither the IfTrue nor the IfFalse nodes
+          // have users except for the Merge and the Merge has no Phi or
+          // EffectPhi uses, so replace the Merge with the control input of the
+          // diamond.
+          TRACE(("  DeadDiamond: #%d:%s #%d:%s #%d:%s\n", node0->id(),
+                 node0->op()->mnemonic(), node1->id(), node1->op()->mnemonic(),
+                 branch0->id(), branch0->op()->mnemonic()));
+          return NodeProperties::GetControlInput(branch0);
+        }
+      }
     }
-    // Edit the merge in place, removing dead inputs.
-    RemoveDeadInputs(node, node);
+
     return node;
   }