[turbofan] Don't treat the hole NaN as constant inside the compiler.

We use a signaling NaN to represent the hole in
FAST_HOLEY_DOUBLE_ELEMENTS backing stores, but on Intel processors, the
C++ compiler may decide to (or be forced to due to calling conventions)
use X87 registers for double values. However transfering to X87
registers automatically quietens the NaNs and there's no way to disable
this. Therefore we should just always load the hole NaN from the canonical
place identified by the address_of_hole_nan external reference instead,
which might even be more efficient in some cases.

R=jarin@chromium.org, jkummerow@chromium.org
BUG=v8:5332

Review-Url: https://codereview.chromium.org/2303643002
Cr-Commit-Position: refs/heads/master@{#39062}

src/assembler.cc

@@ -120,7 +120,7 @@ double min_int;
 double one_half;
 double minus_one_half;
 double negative_infinity;
-double the_hole_nan;
+uint64_t the_hole_nan;
 double uint32_bias;
 };
 
@@ -928,7 +928,7 @@ void ExternalReference::SetUp() {
   double_constants.min_int = kMinInt;
   double_constants.one_half = 0.5;
   double_constants.minus_one_half = -0.5;
-  double_constants.the_hole_nan = bit_cast<double>(kHoleNanInt64);
+  double_constants.the_hole_nan = kHoleNanInt64;
   double_constants.negative_infinity = -V8_INFINITY;
   double_constants.uint32_bias =
     static_cast<double>(static_cast<uint32_t>(0xFFFFFFFF)) + 1;

src/compiler/access-builder.cc

@@ -14,6 +14,14 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+// static
+FieldAccess AccessBuilder::ForExternalDoubleValue() {
+  FieldAccess access = {kUntaggedBase,          0,
+                        MaybeHandle<Name>(),    Type::Number(),
+                        MachineType::Float64(), kNoWriteBarrier};
+  return access;
+}
+
 // static
 FieldAccess AccessBuilder::ForMap() {
   FieldAccess access = {

src/compiler/access-builder.h

@@ -17,6 +17,12 @@ namespace compiler {
 // parameters to simplified load/store operators.
 class AccessBuilder final : public AllStatic {
  public:
+  // ===========================================================================
+  // Access to external values (based on external references).
+
+  // Provides access to a double field identified by an external reference.
+  static FieldAccess ForExternalDoubleValue();
+
   // ===========================================================================
   // Access to heap object fields and elements (based on tagged pointer).
 

src/compiler/js-create-lowering.cc

@@ -1013,10 +1013,17 @@ Node* JSCreateLowering::AllocateElements(Node* effect, Node* control,
   ElementAccess access = IsFastDoubleElementsKind(elements_kind)
                              ? AccessBuilder::ForFixedDoubleArrayElement()
                              : AccessBuilder::ForFixedArrayElement();
-  Node* value =
-      IsFastDoubleElementsKind(elements_kind)
-          ? jsgraph()->Float64Constant(bit_cast<double>(kHoleNanInt64))
-          : jsgraph()->TheHoleConstant();
+  Node* value;
+  if (IsFastDoubleElementsKind(elements_kind)) {
+    // Load the hole NaN pattern from the canonical location.
+    value = effect = graph()->NewNode(
+        simplified()->LoadField(AccessBuilder::ForExternalDoubleValue()),
+        jsgraph()->ExternalConstant(
+            ExternalReference::address_of_the_hole_nan()),
+        effect, control);
+  } else {
+    value = jsgraph()->TheHoleConstant();
+  }
 
   // Actually allocate the backing store.
   AllocationBuilder a(jsgraph(), effect, control);
@@ -1169,18 +1176,18 @@ Node* JSCreateLowering::AllocateFastLiteralElements(
   if (elements_map->instance_type() == FIXED_DOUBLE_ARRAY_TYPE) {
     Handle<FixedDoubleArray> elements =
         Handle<FixedDoubleArray>::cast(boilerplate_elements);
+    Node* the_hole_value = nullptr;
     for (int i = 0; i < elements_length; ++i) {
       if (elements->is_the_hole(i)) {
-        // TODO(turbofan): We cannot currently safely pass thru the (signaling)
-        // hole NaN in C++ code, as the C++ compiler on Intel might use FPU
-        // instructions/registers for doubles and therefore make the NaN quiet.
-        // We should consider passing doubles in the compiler as raw int64
-        // values to prevent this.
-        elements_values[i] = effect =
-            graph()->NewNode(simplified()->LoadElement(
-                                 AccessBuilder::ForFixedDoubleArrayElement()),
-                             jsgraph()->HeapConstant(elements),
-                             jsgraph()->Constant(i), effect, control);
+        if (the_hole_value == nullptr) {
+          // Load the hole NaN pattern from the canonical location.
+          the_hole_value = effect = graph()->NewNode(
+              simplified()->LoadField(AccessBuilder::ForExternalDoubleValue()),
+              jsgraph()->ExternalConstant(
+                  ExternalReference::address_of_the_hole_nan()),
+              effect, control);
+        }
+        elements_values[i] = the_hole_value;
       } else {
         elements_values[i] = jsgraph()->Constant(elements->get_scalar(i));
       }

test/mjsunit/regress/regress-5332.js

+// Copyright 2016 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() {
+  function foo() {
+    var a = new Array(2);
+    a[1] = 1.5;
+    return a;
+  }
+
+  assertEquals(undefined, foo()[0]);
+  assertEquals(undefined, foo()[0]);
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(undefined, foo()[0]);
+})();
+
+(function() {
+  function foo() {
+    var a = Array(2);
+    a[1] = 1.5;
+    return a;
+  }
+
+  assertEquals(undefined, foo()[0]);
+  assertEquals(undefined, foo()[0]);
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(undefined, foo()[0]);
+})();