AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert of binary-operator-reducer: reduce mul+div(shift) (patchset #11 id:200001 of https://codereview.chromium.org/1350223006/ )

Browse Source 
Reason for revert:
This is also unsound for the reasons outlined in
https://codereview.chromium.org/1473073004/
Will help Fedor to implement a solution based on simplified operators.

Original issue's description:
> binary-operator-reducer: reduce mul+div(shift)
>
> Reduction Input:
>
>     ChangeInt32ToFloat64=>          TruncateFloat64ToInt32
>                          Float64Mul=>
>     ChangeInt32ToFloat64=>          Float64Div=>TruncateFloat64ToInt32
>
> Output:
>
>          =>  TruncateInt64ToInt32
> Int64Mul
>          =>  Int64Shr => TruncateInt64ToInt32
>
> Test code:
>
>     function mul(a, b) {
>       var l = a & 0x3ffffff;
>       var h = b & 0x3ffffff;
>       var m = l * h;
>
>       var rl = m & 0x3ffffff;
>       var rh = (m / 0x4000000) | 0;
>
>       return rl | rh;
>     }
>
>     mul(1, 2);
>     var a0 = mul(0x3ffffff, 0x3ffffff);
>     mul(0x0, 0x0);
>     %OptimizeFunctionOnNextCall(mul);
>     var a1 = mul(0x3ffffff, 0x3ffffff);
>
>     print(a0 + ' == ' + a1);
>
> BUG=
> R=mstarzinger@chromium.org
>
> Committed: https://crrev.com/461e5b49d022335a7fc4e9d172397a4bd48b93d4
> Cr-Commit-Position: refs/heads/master@{#31899}

TBR=mstarzinger@chromium.org,danno@chromium.org,titzer@chromium.org,fedor@indutny.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

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

Cr-Commit-Position: refs/heads/master@{#32313}
This commit is contained in:
bmeurer 2015-11-25 22:14:56 -08:00 committed by Commit bot
parent dc55405992
commit 5d18e93bd6
11 changed files with 0 additions and 304 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
BUILD.gn
View File

@ -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",

128
src/compiler/binary-operator-reducer.cc
View File

@ -1,128 +0,0 @@
// 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

52
src/compiler/binary-operator-reducer.h
View File

@ -1,52 +0,0 @@
// 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_

1
src/compiler/machine-operator-reducer.cc
View File

@ -648,7 +648,6 @@ Reduction MachineOperatorReducer::ReduceTruncateFloat64ToInt32(Node* node) {
Float64Matcher m(node->InputAt(0));
if (m.HasValue()) return ReplaceInt32(DoubleToInt32(m.Value()));
if (m.IsChangeInt32ToFloat64()) return Replace(m.node()->InputAt(0));
if (m.IsRoundInt64ToFloat64()) return Replace(m.node()->InputAt(0));
if (m.IsPhi()) {
Node* const phi = m.node();
DCHECK_EQ(kRepFloat64, RepresentationOf(OpParameter<MachineType>(phi)));

4
src/compiler/pipeline.cc
View File

@ -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();
}
};

94
test/unittests/compiler/binary-operator-reducer-unittest.cc
View File

@ -1,94 +0,0 @@
// 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

12
test/unittests/compiler/machine-operator-reducer-unittest.cc
View File

@ -1646,18 +1646,6 @@ TEST_F(MachineOperatorReducerTest, StoreRepWord16WithWord32SarAndWord32Shl) {
}
}
TEST_F(MachineOperatorReducerTest, RoundPlusTruncate) {
Node* p0 = Parameter(0);
Node* t0 = graph()->NewNode(machine()->RoundInt64ToFloat64(), p0);
Node* t1 = graph()->NewNode(
machine()->TruncateFloat64ToInt32(TruncationMode::kJavaScript), t0);
Reduction r = Reduce(t1);
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(), p0);
}
} // namespace compiler
} // namespace internal
} // namespace v8

3
test/unittests/compiler/node-test-utils.cc
View File

@ -2559,7 +2559,6 @@ IS_BINOP_MATCHER(Word64And)
IS_BINOP_MATCHER(Word64Or)
IS_BINOP_MATCHER(Word64Sar)
IS_BINOP_MATCHER(Word64Shl)
IS_BINOP_MATCHER(Word64Shr)
IS_BINOP_MATCHER(Word64Equal)
IS_BINOP_MATCHER(Int32AddWithOverflow)
IS_BINOP_MATCHER(Int32Add)
@ -2571,7 +2570,6 @@ IS_BINOP_MATCHER(Uint32LessThan)
IS_BINOP_MATCHER(Uint32LessThanOrEqual)
IS_BINOP_MATCHER(Int64Add)
IS_BINOP_MATCHER(Int64Sub)
IS_BINOP_MATCHER(Int64Mul)
IS_BINOP_MATCHER(JSAdd)
IS_BINOP_MATCHER(Float32Max)
IS_BINOP_MATCHER(Float32Min)
@ -2600,7 +2598,6 @@ IS_UNOP_MATCHER(ChangeUint32ToUint64)
IS_UNOP_MATCHER(TruncateFloat64ToFloat32)
IS_UNOP_MATCHER(TruncateFloat64ToInt32)
IS_UNOP_MATCHER(TruncateInt64ToInt32)
IS_UNOP_MATCHER(RoundInt64ToFloat64)
IS_UNOP_MATCHER(Float32Abs)
IS_UNOP_MATCHER(Float64Abs)
IS_UNOP_MATCHER(Float64Sqrt)

5
test/unittests/compiler/node-test-utils.h
View File

@ -280,8 +280,6 @@ Matcher<Node*> IsWord64Or(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsWord64Shl(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsWord64Shr(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsWord64Sar(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsWord64Equal(const Matcher<Node*>& lhs_matcher,
@ -306,8 +304,6 @@ Matcher<Node*> IsInt64Add(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsInt64Sub(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsInt64Mul(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsJSAdd(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsChangeFloat64ToInt32(const Matcher<Node*>& input_matcher);
@ -319,7 +315,6 @@ Matcher<Node*> IsChangeUint32ToUint64(const Matcher<Node*>& input_matcher);
Matcher<Node*> IsTruncateFloat64ToFloat32(const Matcher<Node*>& input_matcher);
Matcher<Node*> IsTruncateFloat64ToInt32(const Matcher<Node*>& input_matcher);
Matcher<Node*> IsTruncateInt64ToInt32(const Matcher<Node*>& input_matcher);
Matcher<Node*> IsRoundInt64ToFloat64(const Matcher<Node*>& input_matcher);
Matcher<Node*> IsFloat32Max(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsFloat32Min(const Matcher<Node*>& lhs_matcher,

1
test/unittests/unittests.gyp
View File

@ -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',

2
tools/gyp/v8.gyp
View File

@ -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',