Commit 9dff8185 authored by Benedikt Meurer's avatar Benedikt Meurer

Revert "[turbofan] Make Factory::NewNumber() always return the minus_zero_value."

This reverts commit f578d35b because
(immutable) heap numbers are surprisingly mutable in V8. Someone else
might want to cleanup the mess, otherwise we'll just keep it this way.

TBR=jkummerow@chromium.org
BUG=chromium:454894
LOG=y

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

Cr-Commit-Position: refs/heads/master@{#26447}
parent dbc026ee
......@@ -1018,7 +1018,7 @@ Handle<Object> Factory::NewNumber(double value,
// We need to distinguish the minus zero value and this cannot be
// done after conversion to int. Doing this by comparing bit
// patterns is faster than using fpclassify() et al.
if (IsMinusZero(value)) return minus_zero_value();
if (IsMinusZero(value)) return NewHeapNumber(-0.0, IMMUTABLE, pretenure);
int int_value = FastD2IChecked(value);
if (value == int_value && Smi::IsValid(int_value)) {
......
......@@ -2508,19 +2508,6 @@ MaybeHandle<Object> BinaryOpIC::Transition(
ASSIGN_RETURN_ON_EXCEPTION(
isolate(), result, Execution::Call(isolate(), function, left, 1, &right),
Object);
if (result->IsHeapNumber()) {
// If the result of this BinaryOpIC is used as left or right hand side of
// another binary operation, full-codegen.cc might have decided that its
// safe to reuse the double box returned by this BinaryOpIC, but the builtin
// above does not know or care about this fact and might return a canonical
// value (i.e. the global minus zero constant), which we would then
// overwrite in the surrounding binary operation. So to be safe, we need to
// take a copy of heap numbers here.
result = isolate()->factory()->NewHeapNumber(result->Number());
}
DCHECK(!result.is_identical_to(isolate()->factory()->nan_value()));
DCHECK(!result.is_identical_to(isolate()->factory()->infinity_value()));
DCHECK(!result.is_identical_to(isolate()->factory()->minus_zero_value()));
// Execution::Call can execute arbitrary JavaScript, hence potentially
// update the state of this very IC, so we must update the stored state.
......
// 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/factory.h"
#include "src/handles-inl.h"
#include "test/unittests/test-utils.h"
#include "testing/gmock-support.h"
using testing::BitEq;
namespace v8 {
namespace internal {
typedef TestWithIsolate FactoryTest;
namespace {
const PretenureFlag kPretenureFlags[] = {TENURED, NOT_TENURED};
} // namespace
TEST_F(FactoryTest, MinusZeroValue) {
Handle<Object> minus_zero_value = factory()->minus_zero_value();
EXPECT_TRUE(minus_zero_value->IsHeapNumber());
EXPECT_THAT(minus_zero_value->Number(), BitEq(-0.0));
}
TEST_F(FactoryTest, NewNumberWithMinusZero) {
Handle<Object> minus_zero_value = factory()->minus_zero_value();
TRACED_FOREACH(PretenureFlag, pretenure_flag, kPretenureFlags) {
EXPECT_TRUE(minus_zero_value.is_identical_to(
factory()->NewNumber(-0.0, pretenure_flag)));
}
}
TEST_F(FactoryTest, NewHeapNumberWithMinusZero) {
TRACED_FOREACH(PretenureFlag, pretenure_flag, kPretenureFlags) {
Handle<Object> value =
factory()->NewHeapNumber(-0.0, IMMUTABLE, pretenure_flag);
EXPECT_TRUE(value->IsHeapNumber());
EXPECT_THAT(value->Number(), BitEq(-0.0));
EXPECT_FALSE(value.is_identical_to(factory()->minus_zero_value()));
}
}
} // namespace internal
} // namespace v8
......@@ -75,7 +75,6 @@
'compiler/simplified-operator-unittest.cc',
'compiler/value-numbering-reducer-unittest.cc',
'compiler/zone-pool-unittest.cc',
'factory-unittest.cc',
'libplatform/default-platform-unittest.cc',
'libplatform/task-queue-unittest.cc',
'libplatform/worker-thread-unittest.cc',
......
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