[turbofan] Unify RedundancyElimination for speculative number operations.

Remove the NumberConstant right hand side limitation for the speculative
number operation optimization, and extend the logic to also deal with
SpeculativeToNumber, which is common when dealing with postfix increment
and array operations.

Also add appropriate tests for all the relevant cases, specifically we
mjsunit tests to increase the general coverage for the various cases
here (in addition to dedicated unittests).

Bug: v8:8015
Change-Id: I8c92f98490c63b07eb19686efd404322979e57c4
Reviewed-on: https://chromium-review.googlesource.com/1235919Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56072}

src/compiler/redundancy-elimination.cc

@@ -40,9 +40,8 @@ Reduction RedundancyElimination::Reduce(Node* node) {
     case IrOpcode::kSpeculativeNumberSubtract:
     case IrOpcode::kSpeculativeSafeIntegerAdd:
     case IrOpcode::kSpeculativeSafeIntegerSubtract:
-      // For increments and decrements by a constant, try to learn from the last
-      // bounds check.
-      return TryReuseBoundsCheckForFirstInput(node);
+    case IrOpcode::kSpeculativeToNumber:
+      return ReduceSpeculativeNumberOperation(node);
     case IrOpcode::kEffectPhi:
       return ReduceEffectPhi(node);
     case IrOpcode::kDead:
@@ -242,38 +241,6 @@ Reduction RedundancyElimination::ReduceCheckNode(Node* node) {
   return UpdateChecks(node, checks->AddCheck(zone(), node));
 }
 
-Reduction RedundancyElimination::TryReuseBoundsCheckForFirstInput(Node* node) {
-  DCHECK(node->opcode() == IrOpcode::kSpeculativeNumberAdd ||
-         node->opcode() == IrOpcode::kSpeculativeNumberSubtract ||
-         node->opcode() == IrOpcode::kSpeculativeSafeIntegerAdd ||
-         node->opcode() == IrOpcode::kSpeculativeSafeIntegerSubtract);
-
-  DCHECK_EQ(1, node->op()->EffectInputCount());
-  DCHECK_EQ(1, node->op()->EffectOutputCount());
-
-  Node* const effect = NodeProperties::GetEffectInput(node);
-  EffectPathChecks const* checks = node_checks_.Get(effect);
-
-  // If we do not know anything about the predecessor, do not propagate just yet
-  // because we will have to recompute anyway once we compute the predecessor.
-  if (checks == nullptr) return NoChange();
-
-  Node* left = node->InputAt(0);
-  Node* right = node->InputAt(1);
-  // Only use bounds checks for increments/decrements by a constant.
-  if (right->opcode() == IrOpcode::kNumberConstant) {
-    if (Node* bounds_check = checks->LookupBoundsCheckFor(left)) {
-      // Only use the bounds checked type if it is better.
-      if (NodeProperties::GetType(bounds_check)
-              .Is(NodeProperties::GetType(left))) {
-        node->ReplaceInput(0, bounds_check);
-      }
-    }
-  }
-
-  return UpdateChecks(node, checks);
-}
-
 Reduction RedundancyElimination::ReduceEffectPhi(Node* node) {
   Node* const control = NodeProperties::GetControlInput(node);
   if (control->opcode() == IrOpcode::kLoop) {
@@ -302,6 +269,39 @@ Reduction RedundancyElimination::ReduceEffectPhi(Node* node) {
   return UpdateChecks(node, checks);
 }
 
+Reduction RedundancyElimination::ReduceSpeculativeNumberOperation(Node* node) {
+  DCHECK(node->opcode() == IrOpcode::kSpeculativeNumberAdd ||
+         node->opcode() == IrOpcode::kSpeculativeNumberSubtract ||
+         node->opcode() == IrOpcode::kSpeculativeSafeIntegerAdd ||
+         node->opcode() == IrOpcode::kSpeculativeSafeIntegerSubtract ||
+         node->opcode() == IrOpcode::kSpeculativeToNumber);
+  DCHECK_EQ(1, node->op()->EffectInputCount());
+  DCHECK_EQ(1, node->op()->EffectOutputCount());
+
+  Node* const first = NodeProperties::GetValueInput(node, 0);
+  Node* const effect = NodeProperties::GetEffectInput(node);
+  EffectPathChecks const* checks = node_checks_.Get(effect);
+  // If we do not know anything about the predecessor, do not propagate just yet
+  // because we will have to recompute anyway once we compute the predecessor.
+  if (checks == nullptr) return NoChange();
+
+  // Check if there's a CheckBounds operation on {first}
+  // in the graph already, which we might be able to
+  // reuse here to improve the representation selection
+  // for the {node} later on.
+  if (Node* check = checks->LookupBoundsCheckFor(first)) {
+    // Only use the bounds {check} if its type is better
+    // than the type of the {first} node, otherwise we
+    // would end up replacing NumberConstant inputs with
+    // CheckBounds operations, which is kind of pointless.
+    if (!NodeProperties::GetType(first).Is(NodeProperties::GetType(check))) {
+      NodeProperties::ReplaceValueInput(node, check, 0);
+    }
+  }
+
+  return UpdateChecks(node, checks);
+}
+
 Reduction RedundancyElimination::ReduceStart(Node* node) {
   return UpdateChecks(node, EffectPathChecks::Empty(zone()));
 }

src/compiler/redundancy-elimination.h

@@ -59,14 +59,13 @@ class V8_EXPORT_PRIVATE RedundancyElimination final : public AdvancedReducer {
 
   Reduction ReduceCheckNode(Node* node);
   Reduction ReduceEffectPhi(Node* node);
+  Reduction ReduceSpeculativeNumberOperation(Node* node);
   Reduction ReduceStart(Node* node);
   Reduction ReduceOtherNode(Node* node);
 
   Reduction TakeChecksFromFirstEffect(Node* node);
   Reduction UpdateChecks(Node* node, EffectPathChecks const* checks);
 
-  Reduction TryReuseBoundsCheckForFirstInput(Node* node);
-
   Zone* zone() const { return zone_; }
 
   PathChecksForEffectNodes node_checks_;

test/mjsunit/compiler/redundancy-elimination.js

+// Copyright 2018 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
+
+// Test the RedundancyElimination::ReduceSpeculativeNumberOperation()
+// TurboFan optimization for the case of SpeculativeNumberAdd with
+// Number feedback.
+(function() {
+  function bar(i) {
+    return ++i;
+  }
+  bar(0.1);
+
+  function foo(a, i) {
+    const x = a[i];
+    const y = a[bar(i)];
+    return x + y;
+  }
+
+  assertEquals(3, foo([1, 2], 0));
+  assertEquals(3, foo([1, 2], 0));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(3, foo([1, 2], 0));
+})();
+
+// Test the RedundancyElimination::ReduceSpeculativeNumberOperation()
+// TurboFan optimization for the case of SpeculativeNumberAdd with
+// NumberOrOddball feedback.
+(function() {
+  function bar(i) {
+    return ++i;
+  }
+  assertEquals(NaN, bar(undefined));
+
+  function foo(a, i) {
+    const x = a[i];
+    const y = a[bar(i)];
+    return x + y;
+  }
+
+  assertEquals(3, foo([1, 2], 0));
+  assertEquals(3, foo([1, 2], 0));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(3, foo([1, 2], 0));
+})();
+
+// Test the RedundancyElimination::ReduceSpeculativeNumberOperation()
+// TurboFan optimization for the case of SpeculativeNumberSubtract with
+// Number feedback.
+(function() {
+  function bar(i) {
+    return --i;
+  }
+  assertEquals(-0.9, bar(0.1));
+
+  function foo(a, i) {
+    const x = a[i];
+    const y = a[bar(i)];
+    return x + y;
+  }
+
+  assertEquals(3, foo([1, 2], 1));
+  assertEquals(3, foo([1, 2], 1));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(3, foo([1, 2], 1));
+})();
+
+// Test the RedundancyElimination::ReduceSpeculativeNumberOperation()
+// TurboFan optimization for the case of SpeculativeNumberSubtract with
+// NumberOrOddball feedback.
+(function() {
+  function bar(i) {
+    return --i;
+  }
+  assertEquals(NaN, bar(undefined));
+
+  function foo(a, i) {
+    const x = a[i];
+    const y = a[bar(i)];
+    return x + y;
+  }
+
+  assertEquals(3, foo([1, 2], 1));
+  assertEquals(3, foo([1, 2], 1));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(3, foo([1, 2], 1));
+})();
+
+// Test the RedundancyElimination::ReduceSpeculativeNumberOperation()
+// TurboFan optimization for the case of SpeculativeToNumber.
+(function() {
+  function foo(a, i) {
+    const x = a[i];
+    const y = i++;
+    return x + y;
+  }
+
+  assertEquals(1, foo([1, 2], 0));
+  assertEquals(1, foo([1, 2], 0));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(1, foo([1, 2], 0));
+})();
+
+// Test the RedundancyElimination::ReduceSpeculativeNumberOperation()
+// TurboFan optimization for the case of SpeculativeSafeIntegerAdd.
+(function() {
+  function foo(a, i) {
+    const x = a[i];
+    const y = a[++i];
+    return x + y;
+  }
+
+  assertEquals(3, foo([1, 2], 0));
+  assertEquals(3, foo([1, 2], 0));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(3, foo([1, 2], 0));
+})();
+
+// Test the RedundancyElimination::ReduceSpeculativeNumberOperation()
+// TurboFan optimization for the case of SpeculativeSafeIntegerSubtract.
+(function() {
+  function foo(a, i) {
+    const x = a[i];
+    const y = a[--i];
+    return x + y;
+  }
+
+  assertEquals(3, foo([1, 2], 1));
+  assertEquals(3, foo([1, 2], 1));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(3, foo([1, 2], 1));
+})();

test/unittests/compiler/graph-unittest.cc

@@ -34,6 +34,11 @@ Node* GraphTest::Parameter(int32_t index) {
   return graph()->NewNode(common()->Parameter(index), graph()->start());
 }
 
+Node* GraphTest::Parameter(Type type, int32_t index) {
+  Node* node = GraphTest::Parameter(index);
+  NodeProperties::SetType(node, type);
+  return node;
+}
 
 Node* GraphTest::Float32Constant(volatile float value) {
   return graph()->NewNode(common()->Float32Constant(value));
@@ -117,12 +122,6 @@ TypedGraphTest::TypedGraphTest(int num_parameters)
 
 TypedGraphTest::~TypedGraphTest() = default;
 
-Node* TypedGraphTest::Parameter(Type type, int32_t index) {
-  Node* node = GraphTest::Parameter(index);
-  NodeProperties::SetType(node, type);
-  return node;
-}
-
 namespace graph_unittest {
 
 const Operator kDummyOperator(0, Operator::kNoProperties, "Dummy", 0, 0, 0, 1,

test/unittests/compiler/graph-unittest.h

@@ -34,6 +34,7 @@ class GraphTest : public virtual TestWithNativeContext,
   Node* end() { return graph()->end(); }
 
   Node* Parameter(int32_t index = 0);
+  Node* Parameter(Type type, int32_t index = 0);
   Node* Float32Constant(volatile float value);
   Node* Float64Constant(volatile double value);
   Node* Int32Constant(int32_t value);
@@ -79,9 +80,6 @@ class TypedGraphTest : public GraphTest {
   ~TypedGraphTest() override;
 
  protected:
-  Node* Parameter(int32_t index = 0) { return GraphTest::Parameter(index); }
-  Node* Parameter(Type type, int32_t index = 0);
-
   Typer* typer() { return &typer_; }
 
  private:

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

@@ -1947,7 +1947,7 @@ Matcher<Node*> IsTailCall(
         IrOpcode::k##opcode, hint_matcher, lhs_matcher, rhs_matcher,          \
         effect_matcher, control_matcher));                                    \
   }
-SPECULATIVE_BINOPS(DEFINE_SPECULATIVE_BINOP_MATCHER);
+SIMPLIFIED_SPECULATIVE_NUMBER_BINOP_LIST(DEFINE_SPECULATIVE_BINOP_MATCHER);
 #undef DEFINE_SPECULATIVE_BINOP_MATCHER
 
 Matcher<Node*> IsStringConcat(const Matcher<Node*>& length_matcher,

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

@@ -7,6 +7,7 @@
 
 #include "src/compiler/common-operator.h"
 #include "src/compiler/machine-operator.h"
+#include "src/compiler/opcodes.h"
 #include "src/compiler/simplified-operator.h"
 #include "src/machine-type.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -35,16 +36,6 @@ class Node;
 
 using ::testing::Matcher;
 
-#define SPECULATIVE_BINOPS(V)           \
-  V(SpeculativeNumberAdd)               \
-  V(SpeculativeNumberSubtract)          \
-  V(SpeculativeNumberShiftLeft)         \
-  V(SpeculativeNumberShiftRight)        \
-  V(SpeculativeNumberShiftRightLogical) \
-  V(SpeculativeNumberBitwiseAnd)        \
-  V(SpeculativeNumberBitwiseOr)         \
-  V(SpeculativeNumberBitwiseXor)
-
 Matcher<Node*> IsDead();
 Matcher<Node*> IsEnd(const Matcher<Node*>& control0_matcher);
 Matcher<Node*> IsEnd(const Matcher<Node*>& control0_matcher,
@@ -221,7 +212,7 @@ Matcher<Node*> IsNumberAdd(const Matcher<Node*>& lhs_matcher,
                             const Matcher<Node*>& rhs_matcher,                \
                             const Matcher<Node*>& effect_matcher,             \
                             const Matcher<Node*>& control_matcher);
-SPECULATIVE_BINOPS(DECLARE_SPECULATIVE_BINOP_MATCHER);
+SIMPLIFIED_SPECULATIVE_NUMBER_BINOP_LIST(DECLARE_SPECULATIVE_BINOP_MATCHER);
 #undef DECLARE_SPECULATIVE_BINOP_MATCHER
 
 Matcher<Node*> IsNumberSubtract(const Matcher<Node*>& lhs_matcher,