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

TEST=unittests

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

Cr-Commit-Position: refs/heads/master@{#26124}

src/factory.cc

@@ -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 NewHeapNumber(-0.0, IMMUTABLE, pretenure);
+  if (IsMinusZero(value)) return minus_zero_value();
 
   int int_value = FastD2IChecked(value);
   if (value == int_value && Smi::IsValid(int_value)) {

src/ic/ic.cc

@@ -2499,6 +2499,19 @@ 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.

test/unittests/factory-unittest.cc

+// 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

test/unittests/unittests.gyp

@@ -69,6 +69,7 @@
         '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',