Introduce Diamond, a helper for building diamond-shaped control patterns.

R=mstarzinger@chromium.org, bmeurer@chromium.org
BUG=

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

Cr-Commit-Position: refs/heads/master@{#25110}
git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25110 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

BUILD.gn

@@ -498,6 +498,7 @@ source_set("v8_base") {
     "src/compiler/control-builders.h",
     "src/compiler/control-reducer.cc",
     "src/compiler/control-reducer.h",
+    "src/compiler/diamond.h",
     "src/compiler/frame.h",
     "src/compiler/gap-resolver.cc",
     "src/compiler/gap-resolver.h",

src/compiler/change-lowering.cc

@@ -5,6 +5,7 @@
 #include "src/compiler/change-lowering.h"
 
 #include "src/code-factory.h"
+#include "src/compiler/diamond.h"
 #include "src/compiler/js-graph.h"
 #include "src/compiler/linkage.h"
 #include "src/compiler/machine-operator.h"
@@ -101,20 +102,11 @@ Node* ChangeLowering::LoadHeapNumberValue(Node* value, Node* control) {
 
 
 Reduction ChangeLowering::ChangeBitToBool(Node* val, Node* control) {
-  Node* branch = graph()->NewNode(common()->Branch(), val, control);
-
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-  Node* true_value = jsgraph()->TrueConstant();
-
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-  Node* false_value = jsgraph()->FalseConstant();
-
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-  Node* phi = graph()->NewNode(
-      common()->Phi(static_cast<MachineType>(kTypeBool | kRepTagged), 2),
-      true_value, false_value, merge);
-
-  return Replace(phi);
+  Diamond d(graph(), common(), val);
+  d.Chain(control);
+  MachineType machine_type = static_cast<MachineType>(kTypeBool | kRepTagged);
+  return Replace(d.Phi(machine_type, jsgraph()->TrueConstant(),
+                       jsgraph()->FalseConstant()));
 }
 
 
@@ -140,21 +132,12 @@ Reduction ChangeLowering::ChangeInt32ToTagged(Node* val, Node* control) {
   Node* add = graph()->NewNode(machine()->Int32AddWithOverflow(), val, val);
   Node* ovf = graph()->NewNode(common()->Projection(1), add);
 
-  Node* branch =
-      graph()->NewNode(common()->Branch(BranchHint::kFalse), ovf, control);
-
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+  Diamond d(graph(), common(), ovf, BranchHint::kFalse);
+  d.Chain(control);
   Node* heap_number = AllocateHeapNumberWithValue(
-      graph()->NewNode(machine()->ChangeInt32ToFloat64(), val), if_true);
-
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+      graph()->NewNode(machine()->ChangeInt32ToFloat64(), val), d.if_true);
   Node* smi = graph()->NewNode(common()->Projection(0), add);
-
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-  Node* phi = graph()->NewNode(common()->Phi(kMachAnyTagged, 2), heap_number,
-                               smi, merge);
-
-  return Replace(phi);
+  return Replace(d.Phi(kMachAnyTagged, heap_number, smi));
 }
 
 
@@ -165,24 +148,17 @@ Reduction ChangeLowering::ChangeTaggedToUI32(Node* val, Node* control,
 
   Node* tag = graph()->NewNode(machine()->WordAnd(), val,
                                jsgraph()->IntPtrConstant(kSmiTagMask));
-  Node* branch =
-      graph()->NewNode(common()->Branch(BranchHint::kFalse), tag, control);
 
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+  Diamond d(graph(), common(), tag, BranchHint::kFalse);
+  d.Chain(control);
   const Operator* op = (signedness == kSigned)
                            ? machine()->ChangeFloat64ToInt32()
                            : machine()->ChangeFloat64ToUint32();
-  Node* change = graph()->NewNode(op, LoadHeapNumberValue(val, if_true));
-
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+  Node* load = graph()->NewNode(op, LoadHeapNumberValue(val, d.if_true));
   Node* number = ChangeSmiToInt32(val);
 
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-  Node* phi = graph()->NewNode(
-      common()->Phi((signedness == kSigned) ? kMachInt32 : kMachUint32, 2),
-      change, number, merge);
-
-  return Replace(phi);
+  return Replace(
+      d.Phi((signedness == kSigned) ? kMachInt32 : kMachUint32, load, number));
 }
 
 
@@ -192,20 +168,13 @@ Reduction ChangeLowering::ChangeTaggedToFloat64(Node* val, Node* control) {
 
   Node* tag = graph()->NewNode(machine()->WordAnd(), val,
                                jsgraph()->IntPtrConstant(kSmiTagMask));
-  Node* branch = graph()->NewNode(common()->Branch(), tag, control);
-
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-  Node* load = LoadHeapNumberValue(val, if_true);
-
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+  Diamond d(graph(), common(), tag, BranchHint::kFalse);
+  d.Chain(control);
+  Node* load = LoadHeapNumberValue(val, d.if_true);
   Node* number = graph()->NewNode(machine()->ChangeInt32ToFloat64(),
                                   ChangeSmiToInt32(val));
 
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-  Node* phi =
-      graph()->NewNode(common()->Phi(kMachFloat64, 2), load, number, merge);
-
-  return Replace(phi);
+  return Replace(d.Phi(kMachFloat64, load, number));
 }
 
 
@@ -215,10 +184,8 @@ Reduction ChangeLowering::ChangeUint32ToTagged(Node* val, Node* control) {
 
   Node* cmp = graph()->NewNode(machine()->Uint32LessThanOrEqual(), val,
                                SmiMaxValueConstant());
-  Node* branch =
-      graph()->NewNode(common()->Branch(BranchHint::kTrue), cmp, control);
-
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+  Diamond d(graph(), common(), cmp, BranchHint::kTrue);
+  d.Chain(control);
   Node* smi = graph()->NewNode(
       machine()->WordShl(),
       machine()->Is64()
@@ -226,15 +193,10 @@ Reduction ChangeLowering::ChangeUint32ToTagged(Node* val, Node* control) {
           : val,
       SmiShiftBitsConstant());
 
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
   Node* heap_number = AllocateHeapNumberWithValue(
-      graph()->NewNode(machine()->ChangeUint32ToFloat64(), val), if_false);
-
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-  Node* phi = graph()->NewNode(common()->Phi(kMachAnyTagged, 2), smi,
-                               heap_number, merge);
+      graph()->NewNode(machine()->ChangeUint32ToFloat64(), val), d.if_false);
 
-  return Replace(phi);
+  return Replace(d.Phi(kMachAnyTagged, smi, heap_number));
 }
 
 

src/compiler/diamond.h

+// Copyright 2013 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.
+
+#ifndef V8_COMPILER_DIAMOND_H_
+#define V8_COMPILER_DIAMOND_H_
+
+#include "src/v8.h"
+
+#include "src/compiler/common-operator.h"
+#include "src/compiler/graph-inl.h"
+#include "src/compiler/node.h"
+
+namespace v8 {
+namespace internal {
+namespace compiler {
+
+// A helper to make it easier to build diamond-shaped control patterns.
+struct Diamond {
+  Graph* graph;
+  CommonOperatorBuilder* common;
+  Node* branch;
+  Node* if_true;
+  Node* if_false;
+  Node* merge;
+
+  Diamond(Graph* g, CommonOperatorBuilder* b, Node* cond,
+          BranchHint hint = BranchHint::kNone) {
+    graph = g;
+    common = b;
+    branch = graph->NewNode(common->Branch(hint), cond, graph->start());
+    if_true = graph->NewNode(common->IfTrue(), branch);
+    if_false = graph->NewNode(common->IfFalse(), branch);
+    merge = graph->NewNode(common->Merge(2), if_true, if_false);
+  }
+
+  // Place {this} after {that} in control flow order.
+  void Chain(Diamond& that) { branch->ReplaceInput(1, that.merge); }
+
+  // Place {this} after {that} in control flow order.
+  void Chain(Node* that) { branch->ReplaceInput(1, that); }
+
+  // Nest {this} into either the if_true or if_false branch of {that}.
+  void Nest(Diamond& that, bool if_true) {
+    if (if_true) {
+      branch->ReplaceInput(1, that.if_true);
+      that.merge->ReplaceInput(0, merge);
+    } else {
+      branch->ReplaceInput(1, that.if_false);
+      that.merge->ReplaceInput(1, merge);
+    }
+  }
+
+  Node* Phi(MachineType machine_type, Node* tv, Node* fv) {
+    return graph->NewNode(common->Phi(machine_type, 2), tv, fv, merge);
+  }
+
+  Node* EffectPhi(Node* tv, Node* fv) {
+    return graph->NewNode(common->EffectPhi(2), tv, fv, merge);
+  }
+
+  void OverwriteWithPhi(Node* node, MachineType machine_type, Node* tv,
+                        Node* fv) {
+    DCHECK(node->InputCount() >= 3);
+    node->set_op(common->Phi(machine_type, 2));
+    node->ReplaceInput(0, tv);
+    node->ReplaceInput(1, fv);
+    node->ReplaceInput(2, merge);
+    node->TrimInputCount(3);
+  }
+
+  void OverwriteWithEffectPhi(Node* node, Node* te, Node* fe) {
+    DCHECK(node->InputCount() >= 3);
+    node->set_op(common->EffectPhi(2));
+    node->ReplaceInput(0, te);
+    node->ReplaceInput(1, fe);
+    node->ReplaceInput(2, merge);
+    node->TrimInputCount(3);
+  }
+};
+}
+}
+}  // namespace v8::internal::compiler
+
+#endif  // V8_COMPILER_DIAMOND_H_

src/compiler/js-builtin-reducer.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "src/compiler/diamond.h"
 #include "src/compiler/graph-inl.h"
 #include "src/compiler/js-builtin-reducer.h"
 #include "src/compiler/node-matchers.h"
@@ -106,17 +107,10 @@ Reduction JSBuiltinReducer::ReduceMathAbs(Node* node) {
     // Math.abs(a:number) -> (a > 0 ? a : 0 - a)
     Node* value = r.left();
     Node* zero = jsgraph()->ZeroConstant();
-    Node* control = graph()->start();
-    Node* tag = graph()->NewNode(simplified()->NumberLessThan(), zero, value);
-
-    Node* branch = graph()->NewNode(common()->Branch(), tag, control);
-    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-
+    Node* cmp = graph()->NewNode(simplified()->NumberLessThan(), zero, value);
+    Diamond d(graph(), common(), cmp);
     Node* neg = graph()->NewNode(simplified()->NumberSubtract(), zero, value);
-    value = graph()->NewNode(common()->Phi(kMachNone, 2), value, neg, merge);
-    return Replace(value);
+    return Replace(d.Phi(kMachNone, value, neg));
   }
   return NoChange();
 }
@@ -150,15 +144,9 @@ Reduction JSBuiltinReducer::ReduceMathMax(Node* node) {
     Node* value = r.GetJSCallInput(0);
     for (int i = 1; i < r.GetJSCallArity(); i++) {
       Node* p = r.GetJSCallInput(i);
-      Node* control = graph()->start();
-      Node* tag = graph()->NewNode(simplified()->NumberLessThan(), value, p);
-
-      Node* branch = graph()->NewNode(common()->Branch(), tag, control);
-      Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-      Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-      Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-
-      value = graph()->NewNode(common()->Phi(kMachNone, 2), p, value, merge);
+      Node* cmp = graph()->NewNode(simplified()->NumberLessThan(), value, p);
+      Diamond d(graph(), common(), cmp);
+      value = d.Phi(kMachNone, p, value);
     }
     return Replace(value);
   }

src/compiler/js-intrinsic-builder.cc

@@ -4,6 +4,7 @@
 
 #include "src/compiler/access-builder.h"
 #include "src/compiler/common-operator.h"
+#include "src/compiler/diamond.h"
 #include "src/compiler/generic-node-inl.h"
 #include "src/compiler/js-intrinsic-builder.h"
 #include "src/compiler/js-operator.h"
@@ -68,29 +69,23 @@ ResultAndEffect JSIntrinsicBuilder::BuildMapCheck(Node* object, Node* effect,
   SimplifiedOperatorBuilder simplified(jsgraph_->zone());
 
   Node* is_smi = graph()->NewNode(simplified.ObjectIsSmi(), object);
-  Node* branch = graph()->NewNode(common()->Branch(), is_smi, graph()->start());
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+  Diamond d(graph(), common(), is_smi);
 
   Node* map = graph()->NewNode(simplified.LoadField(AccessBuilder::ForMap()),
-                               object, effect, if_false);
+                               object, effect, d.if_false);
 
   Node* instance_type = graph()->NewNode(
       simplified.LoadField(AccessBuilder::ForMapInstanceType()), map, map,
-      if_false);
+      d.if_false);
 
   Node* has_map_type =
       graph()->NewNode(jsgraph_->machine()->Word32Equal(), instance_type,
                        jsgraph_->Int32Constant(map_type));
 
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
+  Node* phi = d.Phi(static_cast<MachineType>(kTypeBool | kRepTagged),
+                    jsgraph_->FalseConstant(), has_map_type);
 
-  Node* phi =
-      graph()->NewNode(common()->Phi((MachineType)(kTypeBool | kRepTagged), 2),
-                       jsgraph_->FalseConstant(), has_map_type, merge);
-
-  Node* ephi =
-      graph()->NewNode(common()->EffectPhi(2), effect, instance_type, merge);
+  Node* ephi = d.EffectPhi(effect, instance_type);
 
   return ResultAndEffect(phi, ephi);
 }
@@ -112,44 +107,32 @@ ResultAndEffect JSIntrinsicBuilder::BuildGraphFor_ValueOf(
   SimplifiedOperatorBuilder simplified(jsgraph_->zone());
 
   Node* is_smi = graph()->NewNode(simplified.ObjectIsSmi(), object);
-  Node* branch = graph()->NewNode(common()->Branch(), is_smi, graph()->start());
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+
+  Diamond if_is_smi(graph(), common(), is_smi);
 
   Node* map = graph()->NewNode(simplified.LoadField(AccessBuilder::ForMap()),
-                               object, effect, if_false);
+                               object, effect, if_is_smi.if_false);
 
   Node* instance_type = graph()->NewNode(
       simplified.LoadField(AccessBuilder::ForMapInstanceType()), map, map,
-      if_false);
+      if_is_smi.if_false);
 
   Node* is_value =
       graph()->NewNode(jsgraph_->machine()->Word32Equal(), instance_type,
                        jsgraph_->Constant(JS_VALUE_TYPE));
 
-  Node* branch_is_value =
-      graph()->NewNode(common()->Branch(), is_value, if_false);
-  Node* is_value_true = graph()->NewNode(common()->IfTrue(), branch_is_value);
-  Node* is_value_false = graph()->NewNode(common()->IfFalse(), branch_is_value);
+  Diamond if_is_value(graph(), common(), is_value);
+  if_is_value.Nest(if_is_smi, false);
 
   Node* value =
       graph()->NewNode(simplified.LoadField(AccessBuilder::ForValue()), object,
-                       instance_type, is_value_true);
-
-  Node* merge_is_value =
-      graph()->NewNode(common()->Merge(2), is_value_true, is_value_false);
-
-  Node* phi_is_value = graph()->NewNode(common()->Phi((MachineType)kTypeAny, 2),
-                                        value, object, merge_is_value);
-
+                       instance_type, if_is_value.if_true);
 
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, merge_is_value);
+  Node* phi_is_value = if_is_value.Phi(kTypeAny, value, object);
 
-  Node* phi = graph()->NewNode(common()->Phi((MachineType)kTypeAny, 2), object,
-                               phi_is_value, merge);
+  Node* phi = if_is_smi.Phi(kTypeAny, object, phi_is_value);
 
-  Node* ephi =
-      graph()->NewNode(common()->EffectPhi(2), effect, instance_type, merge);
+  Node* ephi = if_is_smi.EffectPhi(effect, instance_type);
 
   return ResultAndEffect(phi, ephi);
 }

src/compiler/machine-operator-reducer.cc

@@ -7,6 +7,7 @@
 #include "src/base/bits.h"
 #include "src/base/division-by-constant.h"
 #include "src/codegen.h"
+#include "src/compiler/diamond.h"
 #include "src/compiler/generic-node-inl.h"
 #include "src/compiler/graph.h"
 #include "src/compiler/js-graph.h"
@@ -640,22 +641,10 @@ Reduction MachineOperatorReducer::ReduceInt32Mod(Node* node) {
 
       Node* check =
           graph()->NewNode(machine()->Int32LessThan(), dividend, zero);
-      Node* branch = graph()->NewNode(common()->Branch(BranchHint::kFalse),
-                                      check, graph()->start());
-
-      Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+      Diamond d(graph(), common(), check, BranchHint::kFalse);
       Node* neg = Int32Sub(zero, Word32And(Int32Sub(zero, dividend), mask));
-
-      Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
       Node* pos = Word32And(dividend, mask);
-
-      Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-
-      DCHECK_EQ(3, node->InputCount());
-      node->set_op(common()->Phi(kMachInt32, 2));
-      node->ReplaceInput(0, neg);
-      node->ReplaceInput(1, pos);
-      node->ReplaceInput(2, merge);
+      d.OverwriteWithPhi(node, kMachInt32, neg, pos);
     } else {
       Node* quotient = Int32Div(dividend, divisor);
       node->set_op(machine()->Int32Sub());

src/compiler/select-lowering.cc

@@ -5,6 +5,7 @@
 #include "src/compiler/select-lowering.h"
 
 #include "src/compiler/common-operator.h"
+#include "src/compiler/diamond.h"
 #include "src/compiler/generic-node-inl.h"
 #include "src/compiler/graph.h"
 
@@ -26,17 +27,13 @@ Reduction SelectLowering::Reduce(Node* node) {
   SelectParameters const p = SelectParametersOf(node->op());
 
   Node* const cond = node->InputAt(0);
-  Node* const control = graph()->start();
 
   // Check if we already have a diamond for this condition.
   auto i = merges_.find(cond);
   if (i == merges_.end()) {
     // Create a new diamond for this condition and remember its merge node.
-    Node* branch = graph()->NewNode(common()->Branch(p.hint()), cond, control);
-    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-    i = merges_.insert(std::make_pair(cond, merge)).first;
+    Diamond d(graph(), common(), cond);
+    i = merges_.insert(std::make_pair(cond, d.merge)).first;
   }
 
   DCHECK_EQ(cond, i->first);

src/compiler/simplified-lowering.cc

@@ -9,6 +9,7 @@
 #include "src/base/bits.h"
 #include "src/code-factory.h"
 #include "src/compiler/common-operator.h"
+#include "src/compiler/diamond.h"
 #include "src/compiler/graph-inl.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/node-properties-inl.h"
@@ -1172,11 +1173,9 @@ void SimplifiedLowering::DoLoadElement(Node* node, MachineType output_type) {
       node->ReplaceInput(2, effect);
       node->ReplaceInput(3, graph()->start());
     } else {
-      Node* branch = graph()->NewNode(common()->Branch(BranchHint::kTrue),
-                                      check, graph()->start());
+      Diamond d(graph(), common(), check, BranchHint::kTrue);
 
-      Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-      Node* load = graph()->NewNode(op, base, index, effect, if_true);
+      Node* load = graph()->NewNode(op, base, index, effect, d.if_true);
       Node* result = load;
       if (output_type & kRepTagged) {
         // TODO(turbofan): This is ugly as hell!
@@ -1187,7 +1186,6 @@ void SimplifiedLowering::DoLoadElement(Node* node, MachineType output_type) {
             changer.GetTaggedRepresentationFor(result, access.machine_type);
       }
 
-      Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
       Node* undefined;
       if (output_type & kRepTagged) {
         DCHECK_EQ(0, access.machine_type & kRepTagged);
@@ -1202,23 +1200,10 @@ void SimplifiedLowering::DoLoadElement(Node* node, MachineType output_type) {
         undefined = jsgraph()->Int32Constant(0);
       }
 
-      Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-      Node* phi = graph()->NewNode(common()->EffectPhi(2), load, effect, merge);
-
       // Replace effect uses of node with the effect phi.
-      for (UseIter i = node->uses().begin(); i != node->uses().end();) {
-        if (NodeProperties::IsEffectEdge(i.edge())) {
-          i = i.UpdateToAndIncrement(phi);
-        } else {
-          ++i;
-        }
-      }
+      NodeProperties::ReplaceWithValue(node, node, d.EffectPhi(load, effect));
 
-      node->set_op(common()->Phi(output_type, 2));
-      node->ReplaceInput(0, result);
-      node->ReplaceInput(1, undefined);
-      node->ReplaceInput(2, merge);
-      node->TrimInputCount(3);
+      d.OverwriteWithPhi(node, output_type, result, undefined);
     }
   }
 }
@@ -1257,21 +1242,10 @@ void SimplifiedLowering::DoStoreElement(Node* node) {
       node->ReplaceInput(1, select);
       node->RemoveInput(2);
     } else {
-      Node* branch =
-          graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
-
-      Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-      Node* store = graph()->NewNode(op, base, index, value, effect, if_true);
-
-      Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-
-      Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-
-      node->set_op(common()->EffectPhi(2));
-      node->ReplaceInput(0, store);
-      node->ReplaceInput(1, effect);
-      node->ReplaceInput(2, merge);
-      node->TrimInputCount(3);
+      Diamond d(graph(), common(), check, BranchHint::kTrue);
+      d.Chain(control);
+      Node* store = graph()->NewNode(op, base, index, value, effect, d.if_true);
+      d.OverwriteWithEffectPhi(node, store, effect);
     }
   }
 }
@@ -1325,34 +1299,20 @@ Node* SimplifiedLowering::Int32Div(Node* const node) {
     return graph()->NewNode(machine()->Int32Div(), lhs, rhs, graph()->start());
   }
 
-  Node* check0 = graph()->NewNode(machine()->Word32Equal(), rhs, zero);
-  Node* branch0 = graph()->NewNode(common()->Branch(BranchHint::kFalse), check0,
-                                   graph()->start());
-
-  Node* if_true0 = graph()->NewNode(common()->IfTrue(), branch0);
-  Node* true0 = zero;
-
-  Node* if_false0 = graph()->NewNode(common()->IfFalse(), branch0);
-  Node* false0 = nullptr;
-  {
-    Node* check1 = graph()->NewNode(machine()->Word32Equal(), rhs,
-                                    jsgraph()->Int32Constant(-1));
-    Node* branch1 = graph()->NewNode(common()->Branch(BranchHint::kFalse),
-                                     check1, if_false0);
-
-    Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1);
-    Node* true1 = graph()->NewNode(machine()->Int32Sub(), zero, lhs);
-
-    Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1);
-    Node* false1 = graph()->NewNode(machine()->Int32Div(), lhs, rhs, if_false1);
+  Diamond if_zero(graph(), common(),
+                  graph()->NewNode(machine()->Word32Equal(), rhs, zero),
+                  BranchHint::kFalse);
 
-    if_false0 = graph()->NewNode(common()->Merge(2), if_true1, if_false1);
-    false0 = graph()->NewNode(common()->Phi(kMachInt32, 2), true1, false1,
-                              if_false0);
-  }
+  Diamond if_minus_one(graph(), common(),
+                       graph()->NewNode(machine()->Word32Equal(), rhs,
+                                        jsgraph()->Int32Constant(-1)),
+                       BranchHint::kFalse);
+  if_minus_one.Nest(if_zero, false);
+  Node* sub = graph()->NewNode(machine()->Int32Sub(), zero, lhs);
+  Node* div =
+      graph()->NewNode(machine()->Int32Div(), lhs, rhs, if_minus_one.if_false);
 
-  Node* merge0 = graph()->NewNode(common()->Merge(2), if_true0, if_false0);
-  return graph()->NewNode(common()->Phi(kMachInt32, 2), true0, false0, merge0);
+  return if_zero.Phi(kMachInt32, zero, if_minus_one.Phi(kMachInt32, sub, div));
 }
 
 
@@ -1368,34 +1328,19 @@ Node* SimplifiedLowering::Int32Mod(Node* const node) {
     return graph()->NewNode(machine()->Int32Mod(), lhs, rhs, graph()->start());
   }
 
-  Node* check0 = graph()->NewNode(machine()->Word32Equal(), rhs, zero);
-  Node* branch0 = graph()->NewNode(common()->Branch(BranchHint::kFalse), check0,
-                                   graph()->start());
-
-  Node* if_true0 = graph()->NewNode(common()->IfTrue(), branch0);
-  Node* true0 = zero;
-
-  Node* if_false0 = graph()->NewNode(common()->IfFalse(), branch0);
-  Node* false0 = nullptr;
-  {
-    Node* check1 = graph()->NewNode(machine()->Word32Equal(), rhs,
-                                    jsgraph()->Int32Constant(-1));
-    Node* branch1 = graph()->NewNode(common()->Branch(BranchHint::kFalse),
-                                     check1, if_false0);
+  Diamond if_zero(graph(), common(),
+                  graph()->NewNode(machine()->Word32Equal(), rhs, zero),
+                  BranchHint::kFalse);
 
-    Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1);
-    Node* true1 = zero;
-
-    Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1);
-    Node* false1 = graph()->NewNode(machine()->Int32Mod(), lhs, rhs, if_false1);
-
-    if_false0 = graph()->NewNode(common()->Merge(2), if_true1, if_false1);
-    false0 = graph()->NewNode(common()->Phi(kMachInt32, 2), true1, false1,
-                              if_false0);
-  }
+  Diamond if_minus_one(graph(), common(),
+                       graph()->NewNode(machine()->Word32Equal(), rhs,
+                                        jsgraph()->Int32Constant(-1)),
+                       BranchHint::kFalse);
+  if_minus_one.Nest(if_zero, false);
+  Node* mod =
+      graph()->NewNode(machine()->Int32Mod(), lhs, rhs, if_minus_one.if_false);
 
-  Node* merge0 = graph()->NewNode(common()->Merge(2), if_true0, if_false0);
-  return graph()->NewNode(common()->Phi(kMachInt32, 2), true0, false0, merge0);
+  return if_zero.Phi(kMachInt32, zero, if_minus_one.Phi(kMachInt32, zero, mod));
 }
 
 
@@ -1412,17 +1357,9 @@ Node* SimplifiedLowering::Uint32Div(Node* const node) {
   }
 
   Node* check = graph()->NewNode(machine()->Word32Equal(), rhs, zero);
-  Node* branch = graph()->NewNode(common()->Branch(BranchHint::kFalse), check,
-                                  graph()->start());
-
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-  Node* vtrue = zero;
-
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-  Node* vfalse = graph()->NewNode(machine()->Uint32Div(), lhs, rhs, if_false);
-
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-  return graph()->NewNode(common()->Phi(kMachUint32, 2), vtrue, vfalse, merge);
+  Diamond d(graph(), common(), check, BranchHint::kFalse);
+  Node* div = graph()->NewNode(machine()->Uint32Div(), lhs, rhs, d.if_false);
+  return d.Phi(kMachUint32, zero, div);
 }
 
 
@@ -1439,17 +1376,9 @@ Node* SimplifiedLowering::Uint32Mod(Node* const node) {
   }
 
   Node* check = graph()->NewNode(machine()->Word32Equal(), rhs, zero);
-  Node* branch = graph()->NewNode(common()->Branch(BranchHint::kFalse), check,
-                                  graph()->start());
-
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-  Node* vtrue = zero;
-
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-  Node* vfalse = graph()->NewNode(machine()->Uint32Mod(), lhs, rhs, if_false);
-
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-  return graph()->NewNode(common()->Phi(kMachUint32, 2), vtrue, vfalse, merge);
+  Diamond d(graph(), common(), check, BranchHint::kFalse);
+  Node* mod = graph()->NewNode(machine()->Uint32Mod(), lhs, rhs, d.if_false);
+  return d.Phi(kMachUint32, zero, mod);
 }
 
 

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

@@ -973,6 +973,7 @@ struct Branch {
 };
 
 
+// TODO(titzer): use the diamonds from src/compiler/diamond.h here.
 struct Diamond {
   Node* branch;
   Node* if_true;

test/unittests/compiler/diamond-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/common-operator.h"
+#include "src/compiler/diamond.h"
+#include "test/unittests/compiler/graph-unittest.h"
+#include "test/unittests/compiler/node-test-utils.h"
+#include "testing/gmock-support.h"
+
+using testing::AllOf;
+using testing::Capture;
+using testing::CaptureEq;
+
+namespace v8 {
+namespace internal {
+namespace compiler {
+
+class DiamondTest : public GraphTest {
+ public:
+  DiamondTest() : GraphTest(5) {}
+};
+
+
+TEST_F(DiamondTest, SimpleDiamond) {
+  Node* p = Parameter(0);
+  Diamond d(graph(), common(), p);
+  EXPECT_THAT(d.branch, IsBranch(p, graph()->start()));
+  EXPECT_THAT(d.if_true, IsIfTrue(d.branch));
+  EXPECT_THAT(d.if_false, IsIfFalse(d.branch));
+  EXPECT_THAT(d.merge, IsMerge(d.if_true, d.if_false));
+}
+
+
+TEST_F(DiamondTest, DiamondChainDiamond) {
+  Node* p0 = Parameter(0);
+  Node* p1 = Parameter(1);
+  Diamond d0(graph(), common(), p0);
+  Diamond d1(graph(), common(), p1);
+  d1.Chain(d0);
+  EXPECT_THAT(d1.branch, IsBranch(p1, d0.merge));
+  EXPECT_THAT(d0.branch, IsBranch(p0, graph()->start()));
+}
+
+
+TEST_F(DiamondTest, DiamondChainNode) {
+  Node* p1 = Parameter(1);
+  Diamond d1(graph(), common(), p1);
+  Node* other = graph()->NewNode(common()->Merge(0));
+  d1.Chain(other);
+  EXPECT_THAT(d1.branch, IsBranch(p1, other));
+}
+
+
+TEST_F(DiamondTest, DiamondChainN) {
+  Node* params[5] = {Parameter(0), Parameter(1), Parameter(2), Parameter(3),
+                     Parameter(4)};
+  Diamond d[5] = {Diamond(graph(), common(), params[0]),
+                  Diamond(graph(), common(), params[1]),
+                  Diamond(graph(), common(), params[2]),
+                  Diamond(graph(), common(), params[3]),
+                  Diamond(graph(), common(), params[4])};
+
+  for (int i = 1; i < 5; i++) {
+    d[i].Chain(d[i - 1]);
+    EXPECT_THAT(d[i].branch, IsBranch(params[i], d[i - 1].merge));
+  }
+}
+
+
+TEST_F(DiamondTest, DiamondNested_true) {
+  Node* p0 = Parameter(0);
+  Node* p1 = Parameter(1);
+  Diamond d0(graph(), common(), p0);
+  Diamond d1(graph(), common(), p1);
+
+  d1.Nest(d0, true);
+
+  EXPECT_THAT(d0.branch, IsBranch(p0, graph()->start()));
+  EXPECT_THAT(d0.if_true, IsIfTrue(d0.branch));
+  EXPECT_THAT(d0.if_false, IsIfFalse(d0.branch));
+  EXPECT_THAT(d0.merge, IsMerge(d1.merge, d0.if_false));
+
+  EXPECT_THAT(d1.branch, IsBranch(p1, d0.if_true));
+  EXPECT_THAT(d1.if_true, IsIfTrue(d1.branch));
+  EXPECT_THAT(d1.if_false, IsIfFalse(d1.branch));
+  EXPECT_THAT(d1.merge, IsMerge(d1.if_true, d1.if_false));
+}
+
+
+TEST_F(DiamondTest, DiamondNested_false) {
+  Node* p0 = Parameter(0);
+  Node* p1 = Parameter(1);
+  Diamond d0(graph(), common(), p0);
+  Diamond d1(graph(), common(), p1);
+
+  d1.Nest(d0, false);
+
+  EXPECT_THAT(d0.branch, IsBranch(p0, graph()->start()));
+  EXPECT_THAT(d0.if_true, IsIfTrue(d0.branch));
+  EXPECT_THAT(d0.if_false, IsIfFalse(d0.branch));
+  EXPECT_THAT(d0.merge, IsMerge(d0.if_true, d1.merge));
+
+  EXPECT_THAT(d1.branch, IsBranch(p1, d0.if_false));
+  EXPECT_THAT(d1.if_true, IsIfTrue(d1.branch));
+  EXPECT_THAT(d1.if_false, IsIfFalse(d1.branch));
+  EXPECT_THAT(d1.merge, IsMerge(d1.if_true, d1.if_false));
+}
+
+
+TEST_F(DiamondTest, DiamondPhis) {
+  Node* p0 = Parameter(0);
+  Node* p1 = Parameter(1);
+  Node* p2 = Parameter(2);
+  Diamond d(graph(), common(), p0);
+
+  MachineType types[] = {kMachAnyTagged, kMachUint32, kMachInt32};
+
+  for (size_t i = 0; i < arraysize(types); i++) {
+    Node* phi = d.Phi(types[i], p1, p2);
+
+    EXPECT_THAT(d.branch, IsBranch(p0, graph()->start()));
+    EXPECT_THAT(d.if_true, IsIfTrue(d.branch));
+    EXPECT_THAT(d.if_false, IsIfFalse(d.branch));
+    EXPECT_THAT(d.merge, IsMerge(d.if_true, d.if_false));
+    EXPECT_THAT(phi, IsPhi(types[i], p1, p2, d.merge));
+  }
+}
+
+
+TEST_F(DiamondTest, DiamondEffectPhis) {
+  Node* p0 = Parameter(0);
+  Node* p1 = Parameter(1);
+  Node* p2 = Parameter(2);
+  Diamond d(graph(), common(), p0);
+
+  Node* phi = d.EffectPhi(p1, p2);
+
+  EXPECT_THAT(d.branch, IsBranch(p0, graph()->start()));
+  EXPECT_THAT(d.if_true, IsIfTrue(d.branch));
+  EXPECT_THAT(d.if_false, IsIfFalse(d.branch));
+  EXPECT_THAT(d.merge, IsMerge(d.if_true, d.if_false));
+  EXPECT_THAT(phi, IsEffectPhi(p1, p2, d.merge));
+}
+
+
+TEST_F(DiamondTest, BranchHint) {
+  Diamond dn(graph(), common(), Parameter(0));
+  CHECK(BranchHint::kNone == BranchHintOf(dn.branch->op()));
+
+  Diamond dt(graph(), common(), Parameter(0), BranchHint::kTrue);
+  CHECK(BranchHint::kTrue == BranchHintOf(dt.branch->op()));
+
+  Diamond df(graph(), common(), Parameter(0), BranchHint::kFalse);
+  CHECK(BranchHint::kFalse == BranchHintOf(df.branch->op()));
+}
+
+
+}  // namespace compiler
+}  // namespace internal
+}  // namespace v8

test/unittests/compiler/node-test-utils.cc

@@ -300,6 +300,45 @@ class IsPhiMatcher FINAL : public NodeMatcher {
 };
 
 
+class IsEffectPhiMatcher FINAL : public NodeMatcher {
+ public:
+  IsEffectPhiMatcher(const Matcher<Node*>& effect0_matcher,
+                     const Matcher<Node*>& effect1_matcher,
+                     const Matcher<Node*>& control_matcher)
+      : NodeMatcher(IrOpcode::kEffectPhi),
+        effect0_matcher_(effect0_matcher),
+        effect1_matcher_(effect1_matcher),
+        control_matcher_(control_matcher) {}
+
+  virtual void DescribeTo(std::ostream* os) const OVERRIDE {
+    NodeMatcher::DescribeTo(os);
+    *os << "), effect0 (";
+    effect0_matcher_.DescribeTo(os);
+    *os << "), effect1 (";
+    effect1_matcher_.DescribeTo(os);
+    *os << ") and control (";
+    control_matcher_.DescribeTo(os);
+    *os << ")";
+  }
+
+  virtual bool MatchAndExplain(Node* node,
+                               MatchResultListener* listener) const OVERRIDE {
+    return (NodeMatcher::MatchAndExplain(node, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetEffectInput(node, 0),
+                                 "effect0", effect0_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetEffectInput(node, 1),
+                                 "effect1", effect1_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetControlInput(node),
+                                 "control", control_matcher_, listener));
+  }
+
+ private:
+  const Matcher<Node*> effect0_matcher_;
+  const Matcher<Node*> effect1_matcher_;
+  const Matcher<Node*> control_matcher_;
+};
+
+
 class IsProjectionMatcher FINAL : public NodeMatcher {
  public:
   IsProjectionMatcher(const Matcher<size_t>& index_matcher,
@@ -880,6 +919,14 @@ Matcher<Node*> IsPhi(const Matcher<MachineType>& type_matcher,
 }
 
 
+Matcher<Node*> IsEffectPhi(const Matcher<Node*>& effect0_matcher,
+                           const Matcher<Node*>& effect1_matcher,
+                           const Matcher<Node*>& merge_matcher) {
+  return MakeMatcher(
+      new IsEffectPhiMatcher(effect0_matcher, effect1_matcher, merge_matcher));
+}
+
+
 Matcher<Node*> IsProjection(const Matcher<size_t>& index_matcher,
                             const Matcher<Node*>& base_matcher) {
   return MakeMatcher(new IsProjectionMatcher(index_matcher, base_matcher));

test/unittests/compiler/node-test-utils.h

@@ -56,6 +56,9 @@ Matcher<Node*> IsPhi(const Matcher<MachineType>& type_matcher,
                      const Matcher<Node*>& value0_matcher,
                      const Matcher<Node*>& value1_matcher,
                      const Matcher<Node*>& merge_matcher);
+Matcher<Node*> IsEffectPhi(const Matcher<Node*>& effect0_matcher,
+                           const Matcher<Node*>& effect1_matcher,
+                           const Matcher<Node*>& merge_matcher);
 Matcher<Node*> IsProjection(const Matcher<size_t>& index_matcher,
                             const Matcher<Node*>& base_matcher);
 Matcher<Node*> IsCall(const Matcher<CallDescriptor*>& descriptor_matcher,

test/unittests/unittests.gyp

@@ -39,6 +39,7 @@
         'compiler/change-lowering-unittest.cc',
         'compiler/common-operator-unittest.cc',
         'compiler/compiler-test-utils.h',
+        'compiler/diamond-unittest.cc',
         'compiler/graph-reducer-unittest.cc',
         'compiler/graph-unittest.cc',
         'compiler/graph-unittest.h',

tools/gyp/v8.gyp

@@ -411,6 +411,7 @@
         '../../src/compiler/control-builders.h',
         '../../src/compiler/control-reducer.cc',
         '../../src/compiler/control-reducer.h',
+        '../../src/compiler/diamond.h',
         '../../src/compiler/frame.h',
         '../../src/compiler/gap-resolver.cc',
         '../../src/compiler/gap-resolver.h',