[compiler] merge binary-operator-reducer

Merge BinaryOperatorReducer into the MachineOperatorReducer class.
It does not need `Revisit()` calls, because the newly inserted nodes are
visited anyway, and there are no other methods that need AdvancedReducer
there.

BUG=
R=titzer@chromium.org

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

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

BUILD.gn

@@ -723,8 +723,6 @@ source_set("v8_base") {
     "src/compiler/ast-loop-assignment-analyzer.h",
     "src/compiler/basic-block-instrumentor.cc",
     "src/compiler/basic-block-instrumentor.h",
-    "src/compiler/binary-operator-reducer.cc",
-    "src/compiler/binary-operator-reducer.h",
     "src/compiler/branch-elimination.cc",
     "src/compiler/branch-elimination.h",
     "src/compiler/bytecode-graph-builder.cc",

src/compiler/binary-operator-reducer.cc

-// 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.
-
-#include "src/compiler/binary-operator-reducer.h"
-
-#include <algorithm>
-
-#include "src/compiler/common-operator.h"
-#include "src/compiler/graph.h"
-#include "src/compiler/machine-operator.h"
-#include "src/compiler/node.h"
-#include "src/compiler/node-matchers.h"
-#include "src/compiler/node-properties.h"
-#include "src/types-inl.h"
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-BinaryOperatorReducer::BinaryOperatorReducer(Editor* editor, Graph* graph,
-                                             CommonOperatorBuilder* common,
-                                             MachineOperatorBuilder* machine)
-    : AdvancedReducer(editor),
-      graph_(graph),
-      common_(common),
-      machine_(machine),
-      dead_(graph->NewNode(common->Dead())) {}
-
-
-Reduction BinaryOperatorReducer::Reduce(Node* node) {
-  switch (node->opcode()) {
-    case IrOpcode::kFloat64Mul:
-      return ReduceFloat52Mul(node);
-    case IrOpcode::kFloat64Div:
-      return ReduceFloat52Div(node);
-    default:
-      break;
-  }
-  return NoChange();
-}
-
-
-Reduction BinaryOperatorReducer::ReduceFloat52Mul(Node* node) {
-  if (!machine()->Is64()) return NoChange();
-
-  Float64BinopMatcher m(node);
-  if (!m.left().IsChangeInt32ToFloat64() ||
-      !m.right().IsChangeInt32ToFloat64()) {
-    return NoChange();
-  }
-
-  Type* type = NodeProperties::GetType(node);
-  Type::RangeType* range = type->GetRange();
-
-  // JavaScript has 52 bit precision in multiplication
-  if (range == nullptr || range->Min() < 0.0 ||
-      range->Max() > 0xFFFFFFFFFFFFFULL) {
-    return NoChange();
-  }
-
-  Node* mul = graph()->NewNode(machine()->Int64Mul(), m.left().InputAt(0),
-                               m.right().InputAt(0));
-  Revisit(mul);
-
-  Type* range_type = Type::Range(range->Min(), range->Max(), graph()->zone());
-
-  // TODO(indutny): Is Type::Number() a proper thing here? It looks like
-  // every other place is using Type:Internal() for int64 values.
-  // Should we off-load range propagation to Typer?
-  NodeProperties::SetType(
-      mul, Type::Intersect(range_type, Type::Number(), graph()->zone()));
-
-  Node* out = graph()->NewNode(machine()->RoundInt64ToFloat64(), mul);
-  return Replace(out);
-}
-
-
-Reduction BinaryOperatorReducer::ReduceFloat52Div(Node* node) {
-  if (!machine()->Is64()) return NoChange();
-
-  Float64BinopMatcher m(node);
-  if (!m.left().IsRoundInt64ToFloat64()) return NoChange();
-
-  // Right value should be positive...
-  if (!m.right().HasValue() || m.right().Value() <= 0) return NoChange();
-
-  // ...integer...
-  int64_t value = static_cast<int64_t>(m.right().Value());
-  if (value != static_cast<int64_t>(m.right().Value())) return NoChange();
-
-  // ...and should be a power of two.
-  if (!base::bits::IsPowerOfTwo64(value)) return NoChange();
-
-  Node* left = m.left().InputAt(0);
-  Type::RangeType* range = NodeProperties::GetType(left)->GetRange();
-
-  // The result should fit into 32bit word
-  int64_t min = static_cast<int64_t>(range->Min()) / value;
-  int64_t max = static_cast<int64_t>(range->Max()) / value;
-  if (min < 0 || max > 0xFFFFFFFLL) {
-    return NoChange();
-  }
-
-  int64_t shift = WhichPowerOf2_64(static_cast<int64_t>(m.right().Value()));
-
-  // Replace division with 64bit right shift
-  Node* shr =
-      graph()->NewNode(machine()->Word64Shr(), left,
-                       graph()->NewNode(common()->Int64Constant(shift)));
-  Revisit(shr);
-
-  Node* out = graph()->NewNode(machine()->RoundInt64ToFloat64(), shr);
-  return Replace(out);
-}
-
-
-Reduction BinaryOperatorReducer::Change(Node* node, Operator const* op,
-                                        Node* a) {
-  node->ReplaceInput(0, a);
-  node->TrimInputCount(1);
-  NodeProperties::ChangeOp(node, op);
-  return Changed(node);
-}
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8

src/compiler/binary-operator-reducer.h

-// 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.
-
-#ifndef V8_COMPILER_BINARY_OPERATOR_REDUCER_H_
-#define V8_COMPILER_BINARY_OPERATOR_REDUCER_H_
-
-#include "src/compiler/graph-reducer.h"
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-// Forward declarations.
-class CommonOperatorBuilder;
-class Graph;
-class MachineOperatorBuilder;
-class Operator;
-
-
-// Performs strength reduction on nodes that have common operators.
-class BinaryOperatorReducer final : public AdvancedReducer {
- public:
-  BinaryOperatorReducer(Editor* editor, Graph* graph,
-                        CommonOperatorBuilder* common,
-                        MachineOperatorBuilder* machine);
-  ~BinaryOperatorReducer() final {}
-
-  Reduction Reduce(Node* node) final;
-
- private:
-  Reduction ReduceFloat52Mul(Node* node);
-  Reduction ReduceFloat52Div(Node* node);
-
-  Reduction Change(Node* node, Operator const* op, Node* a);
-
-  Graph* graph() const { return graph_; }
-  CommonOperatorBuilder* common() const { return common_; }
-  MachineOperatorBuilder* machine() const { return machine_; }
-  Node* dead() const { return dead_; }
-
-  Graph* const graph_;
-  CommonOperatorBuilder* const common_;
-  MachineOperatorBuilder* const machine_;
-  Node* const dead_;
-};
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_COMPILER_BINARY_OPERATOR_REDUCER_H_

src/compiler/machine-operator-reducer.cc

@@ -350,7 +350,7 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
       if (m.IsFoldable()) {  // K * K => K
         return ReplaceFloat64(m.left().Value() * m.right().Value());
       }
-      break;
+      return ReduceFloat52Mul(node);
     }
     case IrOpcode::kFloat64Div: {
       Float64BinopMatcher m(node);
@@ -364,7 +364,7 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
       if (m.IsFoldable()) {  // K / K => K
         return ReplaceFloat64(m.left().Value() / m.right().Value());
       }
-      break;
+      return ReduceFloat52Div(node);
     }
     case IrOpcode::kFloat64Mod: {
       Float64BinopMatcher m(node);
@@ -1080,6 +1080,88 @@ Reduction MachineOperatorReducer::ReduceFloat64Compare(Node* node) {
 }
 
 
+Reduction MachineOperatorReducer::ReduceFloat52Mul(Node* node) {
+  if (!machine()->Is64()) return NoChange();
+
+  Float64BinopMatcher m(node);
+  if (!m.left().IsChangeInt32ToFloat64() ||
+      !m.right().IsChangeInt32ToFloat64()) {
+    return NoChange();
+  }
+
+  Type* type = NodeProperties::GetType(node);
+  Type::RangeType* range = type->GetRange();
+
+  // JavaScript has 52 bit precision in multiplication
+  if (range == nullptr || range->Min() < 0.0 ||
+      range->Max() > 0xFFFFFFFFFFFFFULL) {
+    return NoChange();
+  }
+
+  Node* mul = graph()->NewNode(machine()->Int64Mul(), m.left().InputAt(0),
+                               m.right().InputAt(0));
+
+  Type* range_type = Type::Range(range->Min(), range->Max(), graph()->zone());
+
+  // TODO(indutny): Is Type::Number() a proper thing here? It looks like
+  // every other place is using Type:Internal() for int64 values.
+  // Should we off-load range propagation to Typer?
+  NodeProperties::SetType(
+      mul, Type::Intersect(range_type, Type::Number(), graph()->zone()));
+
+  Node* out = graph()->NewNode(machine()->RoundInt64ToFloat64(), mul);
+  return Replace(out);
+}
+
+
+Reduction MachineOperatorReducer::ReduceFloat52Div(Node* node) {
+  if (!machine()->Is64()) return NoChange();
+
+  Float64BinopMatcher m(node);
+  if (!m.left().IsRoundInt64ToFloat64()) return NoChange();
+
+  // Right value should be positive...
+  if (!m.right().HasValue() || m.right().Value() <= 0) return NoChange();
+
+  // ...integer...
+  int64_t value = static_cast<int64_t>(m.right().Value());
+  if (value != static_cast<int64_t>(m.right().Value())) return NoChange();
+
+  // ...and should be a power of two.
+  if (!base::bits::IsPowerOfTwo64(value)) return NoChange();
+
+  Node* left = m.left().InputAt(0);
+  Type::RangeType* range = NodeProperties::GetType(left)->GetRange();
+
+  // The RoundInt64ToFloat64 input should fit into 52bit integer
+  if (range == nullptr || range->Min() < 0 ||
+      range->Max() > 0xFFFFFFFFFFFFFULL) {
+    return NoChange();
+  }
+
+  // The result should fit into 32bit word
+  int64_t min = static_cast<int64_t>(range->Min()) / value;
+  int64_t max = static_cast<int64_t>(range->Max()) / value;
+  if (min < 0 || max > 0xFFFFFFFLL) {
+    return NoChange();
+  }
+
+  int64_t shift = WhichPowerOf2_64(static_cast<int64_t>(m.right().Value()));
+
+  // Replace division with 64bit right shift
+  Node* shr =
+      graph()->NewNode(machine()->Word64Shr(), left,
+                       graph()->NewNode(common()->Int64Constant(shift)));
+
+  Type* range_type = Type::Range(min, max, graph()->zone());
+  NodeProperties::SetType(
+      shr, Type::Intersect(range_type, Type::Number(), graph()->zone()));
+
+  Node* out = graph()->NewNode(machine()->RoundInt64ToFloat64(), shr);
+  return Replace(out);
+}
+
+
 CommonOperatorBuilder* MachineOperatorReducer::common() const {
   return jsgraph()->common();
 }

src/compiler/machine-operator-reducer.h

@@ -81,6 +81,8 @@ class MachineOperatorReducer final : public Reducer {
   Reduction ReduceFloat64InsertLowWord32(Node* node);
   Reduction ReduceFloat64InsertHighWord32(Node* node);
   Reduction ReduceFloat64Compare(Node* node);
+  Reduction ReduceFloat52Mul(Node* node);
+  Reduction ReduceFloat52Div(Node* node);
 
   Graph* graph() const;
   JSGraph* jsgraph() const { return jsgraph_; }

src/compiler/pipeline.cc

@@ -12,7 +12,6 @@
 #include "src/compiler/ast-graph-builder.h"
 #include "src/compiler/ast-loop-assignment-analyzer.h"
 #include "src/compiler/basic-block-instrumentor.h"
-#include "src/compiler/binary-operator-reducer.h"
 #include "src/compiler/branch-elimination.h"
 #include "src/compiler/bytecode-graph-builder.h"
 #include "src/compiler/change-lowering.h"
@@ -639,14 +638,11 @@ struct SimplifiedLoweringPhase {
     MachineOperatorReducer machine_reducer(data->jsgraph());
     CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
                                          data->common(), data->machine());
-    BinaryOperatorReducer binary_reducer(&graph_reducer, data->graph(),
-                                         data->common(), data->machine());
     AddReducer(data, &graph_reducer, &dead_code_elimination);
     AddReducer(data, &graph_reducer, &simple_reducer);
     AddReducer(data, &graph_reducer, &value_numbering);
     AddReducer(data, &graph_reducer, &machine_reducer);
     AddReducer(data, &graph_reducer, &common_reducer);
-    AddReducer(data, &graph_reducer, &binary_reducer);
     graph_reducer.ReduceGraph();
   }
 };

test/unittests/compiler/binary-operator-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/binary-operator-reducer.h"
-#include "src/compiler/common-operator.h"
-#include "src/compiler/machine-operator.h"
-#include "src/compiler/machine-type.h"
-#include "src/compiler/node-properties.h"
-#include "src/compiler/operator.h"
-#include "src/compiler/simplified-operator.h"
-#include "src/types-inl.h"
-#include "test/unittests/compiler/graph-reducer-unittest.h"
-#include "test/unittests/compiler/graph-unittest.h"
-#include "test/unittests/compiler/node-test-utils.h"
-
-using testing::StrictMock;
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-class BinaryOperatorReducerTest : public TypedGraphTest {
- public:
-  explicit BinaryOperatorReducerTest(int num_parameters = 1)
-      : TypedGraphTest(num_parameters), machine_(zone()), simplified_(zone()) {}
-  ~BinaryOperatorReducerTest() override {}
-
- protected:
-  Reduction Reduce(AdvancedReducer::Editor* editor, Node* node) {
-    BinaryOperatorReducer reducer(editor, graph(), common(), machine());
-    return reducer.Reduce(node);
-  }
-
-  Reduction Reduce(Node* node) {
-    StrictMock<MockAdvancedReducerEditor> editor;
-    return Reduce(&editor, node);
-  }
-
-  MachineOperatorBuilder* machine() { return &machine_; }
-  SimplifiedOperatorBuilder* simplified() { return &simplified_; }
-
- private:
-  MachineOperatorBuilder machine_;
-  SimplifiedOperatorBuilder simplified_;
-};
-
-
-TEST_F(BinaryOperatorReducerTest, Div52OfMul52) {
-  // This reduction applies only to 64bit arch
-  if (!machine()->Is64()) return;
-
-  Node* p0 = Parameter(0);
-  Node* p1 = Parameter(1);
-  Node* t0 = graph()->NewNode(machine()->ChangeInt32ToFloat64(), p0);
-  Node* t1 = graph()->NewNode(machine()->ChangeInt32ToFloat64(), p1);
-
-  Type* mul_range = Type::Range(0x0, 0xFFFFFFFFFFFFFULL, graph()->zone());
-  Node* mul = graph()->NewNode(machine()->Float64Mul(), t0, t1);
-  NodeProperties::SetType(
-      mul, Type::Intersect(mul_range, Type::Number(), graph()->zone()));
-
-  Node* mul_replacement;
-  auto mul_matcher = IsInt64Mul(p0, p1);
-  {
-    StrictMock<MockAdvancedReducerEditor> editor;
-
-    EXPECT_CALL(editor, Revisit(mul_matcher));
-
-    Reduction r = Reduce(&editor, mul);
-    ASSERT_TRUE(r.Changed());
-    mul_replacement = r.replacement();
-    EXPECT_THAT(mul_replacement, IsRoundInt64ToFloat64(mul_matcher));
-  }
-
-  {
-    StrictMock<MockAdvancedReducerEditor> editor;
-
-    Node* power = Float64Constant(0x4000000);
-    Node* div =
-        graph()->NewNode(machine()->Float64Div(), mul_replacement, power);
-
-    auto shr_matcher = IsWord64Shr(mul_matcher, IsInt64Constant(26));
-    EXPECT_CALL(editor, Revisit(shr_matcher));
-
-    Reduction r = Reduce(&editor, div);
-    ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsRoundInt64ToFloat64(shr_matcher));
-  }
-}
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8

test/unittests/compiler/machine-operator-reducer-unittest.cc

@@ -1679,6 +1679,66 @@ TEST_F(MachineOperatorReducerTest, OverflowingRoundPlusTruncate) {
   ASSERT_TRUE(!r.Changed());
 }
 
+
+TEST_F(MachineOperatorReducerTest, Div52OfMul52) {
+  // This reduction applies only to 64bit arch
+  if (!machine()->Is64()) return;
+
+  Node* p0 = Parameter(0);
+  Node* p1 = Parameter(1);
+  Node* t0 = graph()->NewNode(machine()->ChangeInt32ToFloat64(), p0);
+  Node* t1 = graph()->NewNode(machine()->ChangeInt32ToFloat64(), p1);
+
+  int64_t max = 0xFFFFFFFFFFFFFULL;
+  Type* mul_range = Type::Range(0x0, max, graph()->zone());
+  Node* mul = graph()->NewNode(machine()->Float64Mul(), t0, t1);
+  NodeProperties::SetType(
+      mul, Type::Intersect(mul_range, Type::Number(), graph()->zone()));
+
+  Node* mul_replacement;
+  auto mul_matcher = IsInt64Mul(p0, p1);
+
+  Reduction r = Reduce(mul);
+  ASSERT_TRUE(r.Changed());
+  mul_replacement = r.replacement();
+  EXPECT_THAT(mul_replacement, IsRoundInt64ToFloat64(mul_matcher));
+
+  Node* power = Float64Constant(0x4000000);
+  Node* div = graph()->NewNode(machine()->Float64Div(), mul_replacement, power);
+
+  auto shr_matcher = IsWord64Shr(mul_matcher, IsInt64Constant(26));
+
+  r = Reduce(div);
+  ASSERT_TRUE(r.Changed());
+  EXPECT_THAT(r.replacement(), IsRoundInt64ToFloat64(shr_matcher));
+
+  // It should set `shr` range for reduction of RoundInt64ToFloat64
+  Type* type = NodeProperties::GetType(r.replacement()->InputAt(0));
+  Type::RangeType* range = type->GetRange();
+  ASSERT_TRUE(range != nullptr && range->Min() == 0 &&
+              range->Max() == max / 0x4000000);
+}
+
+
+TEST_F(MachineOperatorReducerTest, Div52OfOverflowingValue) {
+  // This reduction applies only to 64bit arch
+  if (!machine()->Is64()) return;
+
+  Node* p0 = Parameter(0);
+  Node* t0 = graph()->NewNode(machine()->RoundInt64ToFloat64(), p0);
+
+  int64_t max = 0xFFFFFFFFFFFFFFULL;
+  Type* p0_range = Type::Range(0x0, max, graph()->zone());
+  NodeProperties::SetType(
+      p0, Type::Intersect(p0_range, Type::Number(), graph()->zone()));
+
+  Node* power = Float64Constant(0x4000000);
+  Node* div = graph()->NewNode(machine()->Float64Div(), t0, power);
+
+  Reduction r = Reduce(div);
+  ASSERT_TRUE(!r.Changed());
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

test/unittests/unittests.gyp

@@ -44,7 +44,6 @@
         'base/utils/random-number-generator-unittest.cc',
         'cancelable-tasks-unittest.cc',
         'char-predicates-unittest.cc',
-        'compiler/binary-operator-reducer-unittest.cc',
         'compiler/branch-elimination-unittest.cc',
         'compiler/bytecode-graph-builder-unittest.cc',
         'compiler/change-lowering-unittest.cc',

tools/gyp/v8.gyp

@@ -461,8 +461,6 @@
         '../../src/compiler/ast-loop-assignment-analyzer.h',
         '../../src/compiler/basic-block-instrumentor.cc',
         '../../src/compiler/basic-block-instrumentor.h',
-        '../../src/compiler/binary-operator-reducer.cc',
-        '../../src/compiler/binary-operator-reducer.h',
         '../../src/compiler/branch-elimination.cc',
         '../../src/compiler/branch-elimination.h',
         '../../src/compiler/bytecode-graph-builder.cc',