[turbofan] Match selects in control reducer (configurable).

R=bmeurer@chromium.org
BUG=

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

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

src/compiler/control-reducer.cc

@@ -56,7 +56,8 @@ class ControlReducerImpl {
         common_(common),
         state_(jsgraph->graph()->NodeCount(), kUnvisited, zone_),
         stack_(zone_),
-        revisit_(zone_) {}
+        revisit_(zone_),
+        max_phis_for_select_(0) {}
 
   Zone* zone_;
   JSGraph* jsgraph_;
@@ -64,6 +65,7 @@ class ControlReducerImpl {
   ZoneVector<VisitState> state_;
   ZoneDeque<Node*> stack_;
   ZoneDeque<Node*> revisit_;
+  int max_phis_for_select_;
 
   void Reduce() {
     Push(graph()->end());
@@ -536,7 +538,7 @@ class ControlReducerImpl {
     if (live == 0) return dead();  // no remaining inputs.
 
     // Gather phis and effect phis to be edited.
-    ZoneVector<Node*> phis(zone_);
+    NodeVector phis(zone_);
     for (Node* const use : node->uses()) {
       if (NodeProperties::IsPhi(use)) phis.push_back(use);
     }
@@ -564,25 +566,49 @@ class ControlReducerImpl {
 
     DCHECK_EQ(live, node->InputCount());
 
-    // Check if it's an unused diamond.
-    if (live == 2 && phis.empty()) {
+    // Try to remove dead diamonds or introduce selects.
+    if (live == 2 && CheckPhisForSelect(phis)) {
       DiamondMatcher matcher(node);
       if (matcher.Matched() && matcher.IfProjectionsAreOwned()) {
-        // It's a dead diamond, i.e. neither the IfTrue nor the IfFalse nodes
-        // have uses except for the Merge and the Merge has no Phi or
-        // EffectPhi uses, so replace the Merge with the control input of the
-        // diamond.
+        // Dead diamond, i.e. neither the IfTrue nor the IfFalse nodes
+        // have uses except for the Merge. Remove the branch if there
+        // are no phis or replace phis with selects.
+        Node* control = NodeProperties::GetControlInput(matcher.Branch());
+        if (phis.size() == 0) {
+          // No phis. Remove the branch altogether.
           TRACE("  DeadDiamond: #%d:Branch #%d:IfTrue #%d:IfFalse\n",
                 matcher.Branch()->id(), matcher.IfTrue()->id(),
                 matcher.IfFalse()->id());
-        // TODO(turbofan): replace single-phi diamonds with selects.
-        return NodeProperties::GetControlInput(matcher.Branch());
+          return control;
+        } else {
+          // A small number of phis. Replace with selects.
+          Node* cond = matcher.Branch()->InputAt(0);
+          for (Node* phi : phis) {
+            Node* select = graph()->NewNode(
+                common_->Select(OpParameter<MachineType>(phi),
+                                BranchHintOf(matcher.Branch()->op())),
+                cond, matcher.TrueInputOf(phi), matcher.FalseInputOf(phi));
+            TRACE("  MatchSelect: #%d:Branch #%d:IfTrue #%d:IfFalse -> #%d\n",
+                  matcher.Branch()->id(), matcher.IfTrue()->id(),
+                  matcher.IfFalse()->id(), select->id());
+            ReplaceNode(phi, select);
+          }
+          return control;
+        }
       }
     }
 
     return node;
   }
 
+  bool CheckPhisForSelect(const NodeVector& phis) {
+    if (phis.size() > static_cast<size_t>(max_phis_for_select_)) return false;
+    for (Node* phi : phis) {
+      if (phi->opcode() != IrOpcode::kPhi) return false;  // no EffectPhis.
+    }
+    return true;
+  }
+
   // Reduce if projections if the branch has a constant input.
   Node* ReduceIfProjection(Node* node, Decision decision) {
     Node* branch = node->InputAt(0);
@@ -638,8 +664,10 @@ class ControlReducerImpl {
 
 
 void ControlReducer::ReduceGraph(Zone* zone, JSGraph* jsgraph,
-                                 CommonOperatorBuilder* common) {
+                                 CommonOperatorBuilder* common,
+                                 int max_phis_for_select) {
   ControlReducerImpl impl(zone, jsgraph, common);
+  impl.max_phis_for_select_ = max_phis_for_select;
   impl.Reduce();
 }
 
@@ -651,9 +679,11 @@ void ControlReducer::TrimGraph(Zone* zone, JSGraph* jsgraph) {
 
 
 Node* ControlReducer::ReduceMerge(JSGraph* jsgraph,
-                                  CommonOperatorBuilder* common, Node* node) {
+                                  CommonOperatorBuilder* common, Node* node,
+                                  int max_phis_for_select) {
   Zone zone;
   ControlReducerImpl impl(&zone, jsgraph, common);
+  impl.max_phis_for_select_ = max_phis_for_select;
   return impl.ReduceMerge(node);
 }
 

src/compiler/control-reducer.h

@@ -23,14 +23,15 @@ class ControlReducer {
  public:
   // Perform branch folding and dead code elimination on the graph.
   static void ReduceGraph(Zone* zone, JSGraph* graph,
-                          CommonOperatorBuilder* builder);
+                          CommonOperatorBuilder* builder,
+                          int max_phis_for_select = 0);
 
   // Trim nodes in the graph that are not reachable from end.
   static void TrimGraph(Zone* zone, JSGraph* graph);
 
   // Reduces a single merge node and attached phis.
   static Node* ReduceMerge(JSGraph* graph, CommonOperatorBuilder* builder,
-                           Node* node);
+                           Node* node, int max_phis_for_select = 0);
 
   // Testing interface.
   static Node* ReducePhiForTesting(JSGraph* graph,

src/compiler/node-matchers.h

@@ -570,6 +570,20 @@ struct DiamondMatcher : public NodeMatcher {
   Node* IfFalse() const { return if_false_; }
   Node* Merge() const { return node(); }
 
+  Node* TrueInputOf(Node* phi) const {
+    DCHECK(IrOpcode::IsPhiOpcode(phi->opcode()));
+    DCHECK_EQ(3, phi->InputCount());
+    DCHECK_EQ(Merge(), phi->InputAt(2));
+    return phi->InputAt(if_true_ == Merge()->InputAt(0) ? 0 : 1);
+  }
+
+  Node* FalseInputOf(Node* phi) const {
+    DCHECK(IrOpcode::IsPhiOpcode(phi->opcode()));
+    DCHECK_EQ(3, phi->InputCount());
+    DCHECK_EQ(Merge(), phi->InputAt(2));
+    return phi->InputAt(if_true_ == Merge()->InputAt(0) ? 1 : 0);
+  }
+
  private:
   Node* branch_;
   Node* if_true_;

src/compiler/pipeline.cc

@@ -593,22 +593,23 @@ struct ChangeLoweringPhase {
 };
 
 
-struct ControlReductionPhase {
+struct EarlyControlReductionPhase {
+  static const char* phase_name() { return "early control reduction"; }
   void Run(PipelineData* data, Zone* temp_zone) {
     SourcePositionTable::Scope pos(data->source_positions(),
                                    SourcePosition::Unknown());
-    ControlReducer::ReduceGraph(temp_zone, data->jsgraph(), data->common());
+    ControlReducer::ReduceGraph(temp_zone, data->jsgraph(), data->common(), 1);
   }
 };
 
 
-struct EarlyControlReductionPhase : ControlReductionPhase {
-  static const char* phase_name() { return "early control reduction"; }
-};
-
-
-struct LateControlReductionPhase : ControlReductionPhase {
+struct LateControlReductionPhase {
   static const char* phase_name() { return "late control reduction"; }
+  void Run(PipelineData* data, Zone* temp_zone) {
+    SourcePositionTable::Scope pos(data->source_positions(),
+                                   SourcePosition::Unknown());
+    ControlReducer::ReduceGraph(temp_zone, data->jsgraph(), data->common(), 0);
+  }
 };
 
 

test/unittests/compiler/control-reducer-unittest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "src/compiler/control-reducer.h"
+#include "src/compiler/diamond.h"
 #include "src/compiler/graph-visualizer.h"
 #include "src/compiler/js-graph.h"
 #include "src/compiler/js-operator.h"
@@ -34,13 +35,14 @@ class ControlReducerTest : public TypedGraphTest {
   JSOperatorBuilder javascript_;
   JSGraph jsgraph_;
 
-  void ReduceGraph() {
+  void ReduceGraph(int max_phis_for_select = 0) {
     if (FLAG_trace_turbo_graph) {
       OFStream os(stdout);
       os << "-- Graph before control reduction" << std::endl;
       os << AsRPO(*graph());
     }
-    ControlReducer::ReduceGraph(zone(), jsgraph(), common());
+    ControlReducer::ReduceGraph(zone(), jsgraph(), common(),
+                                max_phis_for_select);
     if (FLAG_trace_turbo_graph) {
       OFStream os(stdout);
       os << "-- Graph after control reduction" << std::endl;
@@ -279,6 +281,75 @@ TEST_F(ControlReducerTest, RangeAsInputToBranch_true2) {
 }
 
 
+TEST_F(ControlReducerTest, SelectPhi) {
+  Node* p0 = Parameter(0);
+  const MachineType kType = kMachInt32;
+  Diamond d(graph(), common(), p0);
+  Node* phi =
+      d.Phi(kType, jsgraph()->Int32Constant(1), jsgraph()->Int32Constant(2));
+
+  Node* ret =
+      graph()->NewNode(common()->Return(), phi, graph()->start(), d.merge);
+  graph()->end()->ReplaceInput(0, ret);
+
+  ReduceGraph(1);
+
+  // Phi should be replaced with a select.
+  EXPECT_THAT(graph()->end(),
+              IsEnd(IsReturn(
+                  IsSelect(kType, p0, IsInt32Constant(1), IsInt32Constant(2)),
+                  graph()->start(), graph()->start())));
+}
+
+
+TEST_F(ControlReducerTest, SelectPhis_fail) {
+  Node* p0 = Parameter(0);
+  const MachineType kType = kMachInt32;
+  Diamond d(graph(), common(), p0);
+  Node* phi =
+      d.Phi(kType, jsgraph()->Int32Constant(1), jsgraph()->Int32Constant(2));
+  Node* phi2 =
+      d.Phi(kType, jsgraph()->Int32Constant(11), jsgraph()->Int32Constant(22));
+  USE(phi2);
+  Node* ret =
+      graph()->NewNode(common()->Return(), phi, graph()->start(), d.merge);
+  graph()->end()->ReplaceInput(0, ret);
+
+  ReduceGraph(1);
+
+  // Diamond should not be replaced with a select (too many phis).
+  EXPECT_THAT(ret, IsReturn(phi, graph()->start(), d.merge));
+  EXPECT_THAT(graph()->end(), IsEnd(ret));
+}
+
+
+TEST_F(ControlReducerTest, SelectTwoPhis) {
+  Node* p0 = Parameter(0);
+  const MachineType kType = kMachInt32;
+  Diamond d(graph(), common(), p0);
+  Node* phi1 =
+      d.Phi(kType, jsgraph()->Int32Constant(1), jsgraph()->Int32Constant(2));
+  Node* phi2 =
+      d.Phi(kType, jsgraph()->Int32Constant(2), jsgraph()->Int32Constant(3));
+  MachineOperatorBuilder machine(zone());
+  Node* add = graph()->NewNode(machine.Int32Add(), phi1, phi2);
+  Node* ret =
+      graph()->NewNode(common()->Return(), add, graph()->start(), d.merge);
+  graph()->end()->ReplaceInput(0, ret);
+
+  ReduceGraph(2);
+
+  // Phis should be replaced with two selects.
+  EXPECT_THAT(
+      ret,
+      IsReturn(IsInt32Add(
+                   IsSelect(kType, p0, IsInt32Constant(1), IsInt32Constant(2)),
+                   IsSelect(kType, p0, IsInt32Constant(2), IsInt32Constant(3))),
+               graph()->start(), graph()->start()));
+  EXPECT_THAT(graph()->end(), IsEnd(ret));
+}
+
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8