Commit b8e554d5 authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[turbofan] Properly constant-fold Float64 comparisons.

While investigating crbug.com/878742 I found that somehow the
MachineOperatorReducer lacks the ability to constant-fold
comparisons of Float64 constants, which obviously leads to
pretty weird code.

Bug: v8:8015
Change-Id: I7e18ce10e9d5c87f131fb083ccd3e1e336189dae
Reviewed-on: https://chromium-review.googlesource.com/1226132Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55935}
parent c696376e
......@@ -1379,21 +1379,32 @@ bool IsFloat64RepresentableAsFloat32(const Float64Matcher& m) {
Reduction MachineOperatorReducer::ReduceFloat64Compare(Node* node) {
DCHECK((IrOpcode::kFloat64Equal == node->opcode()) ||
(IrOpcode::kFloat64LessThan == node->opcode()) ||
(IrOpcode::kFloat64LessThanOrEqual == node->opcode()));
// As all Float32 values have an exact representation in Float64, comparing
// two Float64 values both converted from Float32 is equivalent to comparing
// the original Float32s, so we can ignore the conversions. We can also reduce
// comparisons of converted Float64 values against constants that can be
// represented exactly as Float32.
DCHECK(IrOpcode::kFloat64Equal == node->opcode() ||
IrOpcode::kFloat64LessThan == node->opcode() ||
IrOpcode::kFloat64LessThanOrEqual == node->opcode());
Float64BinopMatcher m(node);
if ((m.left().IsChangeFloat32ToFloat64() &&
m.right().IsChangeFloat32ToFloat64()) ||
(m.left().IsChangeFloat32ToFloat64() &&
IsFloat64RepresentableAsFloat32(m.right())) ||
(IsFloat64RepresentableAsFloat32(m.left()) &&
m.right().IsChangeFloat32ToFloat64())) {
if (m.IsFoldable()) {
switch (node->opcode()) {
case IrOpcode::kFloat64Equal:
return ReplaceBool(m.left().Value() == m.right().Value());
case IrOpcode::kFloat64LessThan:
return ReplaceBool(m.left().Value() < m.right().Value());
case IrOpcode::kFloat64LessThanOrEqual:
return ReplaceBool(m.left().Value() <= m.right().Value());
default:
UNREACHABLE();
}
} else if ((m.left().IsChangeFloat32ToFloat64() &&
m.right().IsChangeFloat32ToFloat64()) ||
(m.left().IsChangeFloat32ToFloat64() &&
IsFloat64RepresentableAsFloat32(m.right())) ||
(IsFloat64RepresentableAsFloat32(m.left()) &&
m.right().IsChangeFloat32ToFloat64())) {
// As all Float32 values have an exact representation in Float64, comparing
// two Float64 values both converted from Float32 is equivalent to comparing
// the original Float32s, so we can ignore the conversions. We can also
// reduce comparisons of converted Float64 values against constants that
// can be represented exactly as Float32.
switch (node->opcode()) {
case IrOpcode::kFloat64Equal:
NodeProperties::ChangeOp(node, machine()->Float32Equal());
......@@ -1405,7 +1416,7 @@ Reduction MachineOperatorReducer::ReduceFloat64Compare(Node* node) {
NodeProperties::ChangeOp(node, machine()->Float32LessThanOrEqual());
break;
default:
return NoChange();
UNREACHABLE();
}
node->ReplaceInput(
0, m.left().HasValue()
......
......@@ -2031,6 +2031,16 @@ TEST_F(MachineOperatorReducerTest, Float64InsertHighWord32WithConstant) {
// -----------------------------------------------------------------------------
// Float64Equal
TEST_F(MachineOperatorReducerTest, Float64EqualWithConstant) {
TRACED_FOREACH(double, x, kFloat64Values) {
TRACED_FOREACH(double, y, kFloat64Values) {
Reduction const r = Reduce(graph()->NewNode(
machine()->Float64Equal(), Float64Constant(x), Float64Constant(y)));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(), IsInt32Constant(x == y));
}
}
}
TEST_F(MachineOperatorReducerTest, Float64EqualWithFloat32Conversions) {
Node* const p0 = Parameter(0);
......@@ -2060,6 +2070,17 @@ TEST_F(MachineOperatorReducerTest, Float64EqualWithFloat32Constant) {
// -----------------------------------------------------------------------------
// Float64LessThan
TEST_F(MachineOperatorReducerTest, Float64LessThanWithConstant) {
TRACED_FOREACH(double, x, kFloat64Values) {
TRACED_FOREACH(double, y, kFloat64Values) {
Reduction const r =
Reduce(graph()->NewNode(machine()->Float64LessThan(),
Float64Constant(x), Float64Constant(y)));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(), IsInt32Constant(x < y));
}
}
}
TEST_F(MachineOperatorReducerTest, Float64LessThanWithFloat32Conversions) {
Node* const p0 = Parameter(0);
......@@ -2100,6 +2121,17 @@ TEST_F(MachineOperatorReducerTest, Float64LessThanWithFloat32Constant) {
// -----------------------------------------------------------------------------
// Float64LessThanOrEqual
TEST_F(MachineOperatorReducerTest, Float64LessThanOrEqualWithConstant) {
TRACED_FOREACH(double, x, kFloat64Values) {
TRACED_FOREACH(double, y, kFloat64Values) {
Reduction const r =
Reduce(graph()->NewNode(machine()->Float64LessThanOrEqual(),
Float64Constant(x), Float64Constant(y)));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(), IsInt32Constant(x <= y));
}
}
}
TEST_F(MachineOperatorReducerTest,
Float64LessThanOrEqualWithFloat32Conversions) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment