Skip to content

Commit

Permalink
[turbofan] Reduce consecutive overflow addition with constants
Browse files Browse the repository at this point in the history
Using associative property of addition: (x + A) + B => x + (A + B).
Note: A and B need to have the same sign and we need to check that
(x + A) isn't used anywhere else.

20% perf improvement of the following function.

function f(n) {
  var c = 0;
  for (var i = 0; i < n; i++) {
    c = c + 2 + 3;
  }
  return c;
}
for n = 10_000_000.

Before: 7.31s.
After: 6.05s.

Bug: v8:10305
Change-Id: If45d1cad6128a9a25cb9f43a4828ae28d594a84b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2365221
Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70064}
  • Loading branch information
duongnhn authored and Commit Bot committed Sep 22, 2020
1 parent 2ee48d4 commit e93a369
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 0 deletions.
35 changes: 35 additions & 0 deletions src/compiler/simplified-operator-reducer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,41 @@ Reduction SimplifiedOperatorReducer::Reduce(Node* node) {
if (m.left().node() == m.right().node()) return ReplaceBoolean(true);
break;
}
case IrOpcode::kCheckedInt32Add: {
// (x + a) + b => x + (a + b) where a and b are constants and have the
// same sign.
Int32BinopMatcher m(node);
if (m.right().HasValue()) {
Node* checked_int32_add = m.left().node();
if (checked_int32_add->opcode() == IrOpcode::kCheckedInt32Add) {
Int32BinopMatcher n(checked_int32_add);
if (n.right().HasValue() &&
(n.right().Value() >= 0) == (m.right().Value() >= 0)) {
int32_t val;
bool overflow = base::bits::SignedAddOverflow32(
n.right().Value(), m.right().Value(), &val);
if (!overflow) {
bool has_no_other_value_uses = true;
for (Edge edge : checked_int32_add->use_edges()) {
if (!edge.from()->IsDead() &&
!NodeProperties::IsEffectEdge(edge) &&
edge.from() != node) {
has_no_other_value_uses = false;
break;
}
}
if (has_no_other_value_uses) {
node->ReplaceInput(0, n.left().node());
node->ReplaceInput(1, jsgraph()->Int32Constant(val));
RelaxEffectsAndControls(checked_int32_add);
return Changed(node);
}
}
}
}
}
break;
}
default:
break;
}
Expand Down
30 changes: 30 additions & 0 deletions test/mjsunit/compiler/consecutive-addition.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2020 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

function f1(n) {
var a = n + 2;
return a + 3;
}

%PrepareFunctionForOptimization(f1);
assertEquals(f1(1), 6);
assertEquals(f1(2), 7);
%OptimizeFunctionOnNextCall(f1);
assertEquals(f1(13), 18);
assertEquals(f1(14), 19);

function f2(n, odd) {
var a = n + 2;
if (odd) return a;
return a + 3;
}

%PrepareFunctionForOptimization(f2);
assertEquals(f2(1, true), 3);
assertEquals(f2(2, false), 7);
%OptimizeFunctionOnNextCall(f2);
assertEquals(f2(13, true), 15);
assertEquals(f2(14, false), 19);
57 changes: 57 additions & 0 deletions test/unittests/compiler/simplified-operator-reducer-unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,63 @@ TEST_F(SimplifiedOperatorReducerTest, ObjectIsSmiWithNumberConstant) {
}
}

// -----------------------------------------------------------------------------
// CheckedInt32Add

TEST_F(SimplifiedOperatorReducerTest,
CheckedInt32AddConsecutivelyWithConstants) {
Node* p0 = Parameter(0);
Node* effect = graph()->start();
Node* control = graph()->start();
TRACED_FOREACH(int32_t, a, kInt32Values) {
TRACED_FOREACH(int32_t, b, kInt32Values) {
Node* add1 = graph()->NewNode(simplified()->CheckedInt32Add(), p0,
Int32Constant(a), effect, control);
Node* add2 = graph()->NewNode(simplified()->CheckedInt32Add(), add1,
Int32Constant(b), add1, control);

Reduction r = Reduce(add2);
int32_t c;
bool overflow = base::bits::SignedAddOverflow32(a, b, &c);
if ((a >= 0) == (b >= 0) && !overflow) {
ASSERT_TRUE(r.Changed());
Node* new_node = r.replacement();
ASSERT_EQ(new_node->opcode(), IrOpcode::kCheckedInt32Add);
ASSERT_EQ(new_node->InputAt(0), p0);
EXPECT_THAT(new_node->InputAt(1), IsInt32Constant(c));
ASSERT_EQ(new_node->InputAt(2), effect);
ASSERT_EQ(new_node->InputAt(3), control);
EXPECT_TRUE(add1->uses().empty());
} else {
ASSERT_FALSE(r.Changed());
}
}
}
}

TEST_F(SimplifiedOperatorReducerTest,
CheckedInt32AddConsecutivelyWithConstantsNoChanged) {
Node* p0 = Parameter(0);
Node* effect = graph()->start();
Node* control = graph()->start();
TRACED_FOREACH(int32_t, a, kInt32Values) {
TRACED_FOREACH(int32_t, b, kInt32Values) {
Node* add1 = graph()->NewNode(simplified()->CheckedInt32Add(), p0,
Int32Constant(a), effect, control);
Node* add2 = graph()->NewNode(simplified()->CheckedInt32Add(), add1,
Int32Constant(b), add1, control);
Node* add3 = graph()->NewNode(simplified()->CheckedInt32Add(), add1,
Int32Constant(b), effect, control);

// No changed since add1 has other value uses.
Reduction r = Reduce(add2);
ASSERT_FALSE(r.Changed());
r = Reduce(add3);
ASSERT_FALSE(r.Changed());
}
}
}

} // namespace simplified_operator_reducer_unittest
} // namespace compiler
} // namespace internal
Expand Down

0 comments on commit e93a369

Please sign in to comment.