Merged: [turbofan] Properly represent the float64 hole.

Revision: 8c0c5e8 BUG=chromium:684208,chromium:709753,v8:5267 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true TBR=bmeurer@chromium.org Change-Id: I98d2a7e39892cd186bb8c9a43c6cf87fc6067d85 Reviewed-on: https://chromium-review.googlesource.com/476671 Reviewed-by: Michael Hablich <hablich@chromium.org> Cr-Commit-Position: refs/branch-heads/5.9@{crosswalk-project#6} Cr-Branched-From: fe9bb7e-refs/heads/5.9.211@{crosswalk-project#1} Cr-Branched-From: 70ad237-refs/heads/master@{#44591}
tenta-browser · Apr 13, 2017 · 9372e6d · 9372e6d
1 parent 713e3be
commit 9372e6d
Show file tree

Hide file tree

Showing 15 changed files with 121 additions and 56 deletions.
diff --git a/src/compiler/js-create-lowering.cc b/src/compiler/js-create-lowering.cc
@@ -1103,17 +1103,7 @@ Node* JSCreateLowering::AllocateElements(Node* effect, Node* control,
   ElementAccess access = IsFastDoubleElementsKind(elements_kind)
                              ? AccessBuilder::ForFixedDoubleArrayElement()
                              : AccessBuilder::ForFixedArrayElement();
-  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();
-  }
+  Node* value = jsgraph()->TheHoleConstant();
 
   // Actually allocate the backing store.
   AllocationBuilder a(jsgraph(), effect, control);
@@ -1261,18 +1251,9 @@ 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)) {
-        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;
+        elements_values[i] = jsgraph()->TheHoleConstant();
       } else {
         elements_values[i] = jsgraph()->Constant(elements->get_scalar(i));
       }

diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc
@@ -1962,10 +1962,12 @@ JSNativeContextSpecialization::BuildElementAccess(
     if (access_mode == AccessMode::kLoad) {
       // Compute the real element access type, which includes the hole in case
       // of holey backing stores.
-      if (elements_kind == FAST_HOLEY_ELEMENTS ||
-          elements_kind == FAST_HOLEY_SMI_ELEMENTS) {
+      if (IsHoleyElementsKind(elements_kind)) {
         element_access.type =
             Type::Union(element_type, Type::Hole(), graph()->zone());
+      }
+      if (elements_kind == FAST_HOLEY_ELEMENTS ||
+          elements_kind == FAST_HOLEY_SMI_ELEMENTS) {
         element_access.machine_type = MachineType::AnyTagged();
       }
       // Perform the actual backing store access.

diff --git a/src/compiler/operation-typer.cc b/src/compiler/operation-typer.cc
@@ -1026,6 +1026,19 @@ Type* OperationTyper::FalsifyUndefined(ComparisonOutcome outcome) {
   return singleton_true();
 }
 
+Type* OperationTyper::CheckFloat64Hole(Type* type) {
+  if (type->Maybe(Type::Hole())) {
+    // Turn "the hole" into undefined.
+    type = Type::Intersect(type, Type::Number(), zone());
+    type = Type::Union(type, Type::Undefined(), zone());
+  }
+  return type;
+}
+
+Type* OperationTyper::CheckNumber(Type* type) {
+  return Type::Intersect(type, Type::Number(), zone());
+}
+
 Type* OperationTyper::TypeTypeGuard(const Operator* sigma_op, Type* input) {
   return Type::Intersect(input, TypeGuardTypeOf(sigma_op), zone());
 }

diff --git a/src/compiler/operation-typer.h b/src/compiler/operation-typer.h
@@ -48,6 +48,10 @@ class V8_EXPORT_PRIVATE OperationTyper {
   SIMPLIFIED_SPECULATIVE_NUMBER_BINOP_LIST(DECLARE_METHOD)
 #undef DECLARE_METHOD
 
+  // Check operators.
+  Type* CheckFloat64Hole(Type* type);
+  Type* CheckNumber(Type* type);
+
   Type* TypeTypeGuard(const Operator* sigma_op, Type* input);
 
   enum ComparisonOutcomeFlags {

diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc
@@ -468,6 +468,18 @@ class RepresentationSelector {
         new_type = op_typer_.ToNumber(FeedbackTypeOf(node->InputAt(0)));
         break;
 
+      case IrOpcode::kCheckFloat64Hole:
+        new_type = Type::Intersect(
+            op_typer_.CheckFloat64Hole(FeedbackTypeOf(node->InputAt(0))),
+            info->restriction_type(), graph_zone());
+        break;
+
+      case IrOpcode::kCheckNumber:
+        new_type = Type::Intersect(
+            op_typer_.CheckNumber(FeedbackTypeOf(node->InputAt(0))),
+            info->restriction_type(), graph_zone());
+        break;
+
       case IrOpcode::kPhi: {
         new_type = TypePhi(node);
         if (type != nullptr) {
@@ -819,6 +831,15 @@ class RepresentationSelector {
     if (lower()) Kill(node);
   }
 
+  // Helper for no-op node.
+  void VisitNoop(Node* node, Truncation truncation) {
+    if (truncation.IsUnused()) return VisitUnused(node);
+    MachineRepresentation representation =
+        GetOutputInfoForPhi(node, TypeOf(node), truncation);
+    VisitUnop(node, UseInfo(representation, truncation), representation);
+    if (lower()) DeferReplacement(node, node->InputAt(0));
+  }
+
   // Helper for binops of the R x L -> O variety.
   void VisitBinop(Node* node, UseInfo left_use, UseInfo right_use,
                   MachineRepresentation output,
@@ -2356,19 +2377,9 @@ class RepresentationSelector {
         return;
       }
       case IrOpcode::kCheckNumber: {
-        if (InputIs(node, Type::Number())) {
-          if (truncation.IsUsedAsWord32()) {
-            VisitUnop(node, UseInfo::TruncatingWord32(),
-                      MachineRepresentation::kWord32);
-          } else {
-            // TODO(jarin,bmeurer): We need to go to Tagged here, because
-            // otherwise we cannot distinguish the hole NaN (which might need to
-            // be treated as undefined). We should have a dedicated Type for
-            // that at some point, and maybe even a dedicated truncation.
-            VisitUnop(node, UseInfo::AnyTagged(),
-                      MachineRepresentation::kTagged);
-          }
-          if (lower()) DeferReplacement(node, node->InputAt(0));
+        Type* const input_type = TypeOf(node->InputAt(0));
+        if (input_type->Is(Type::Number())) {
+          VisitNoop(node, truncation);
         } else {
           VisitUnop(node, UseInfo::AnyTagged(), MachineRepresentation::kTagged);
         }
@@ -2681,13 +2692,36 @@ class RepresentationSelector {
         return;
       }
       case IrOpcode::kCheckFloat64Hole: {
-        CheckFloat64HoleMode mode = CheckFloat64HoleModeOf(node->op());
-        ProcessInput(node, 0, UseInfo::TruncatingFloat64());
-        ProcessRemainingInputs(node, 1);
-        SetOutput(node, MachineRepresentation::kFloat64);
-        if (truncation.IsUsedAsFloat64() &&
-            mode == CheckFloat64HoleMode::kAllowReturnHole) {
-          if (lower()) DeferReplacement(node, node->InputAt(0));
+        Type* const input_type = TypeOf(node->InputAt(0));
+        if (input_type->Is(Type::Number())) {
+          VisitNoop(node, truncation);
+        } else {
+          CheckFloat64HoleMode mode = CheckFloat64HoleModeOf(node->op());
+          switch (mode) {
+            case CheckFloat64HoleMode::kAllowReturnHole:
+              if (truncation.IsUnused()) return VisitUnused(node);
+              if (truncation.IsUsedAsWord32()) {
+                VisitUnop(node, UseInfo::TruncatingWord32(),
+                          MachineRepresentation::kWord32);
+                if (lower()) DeferReplacement(node, node->InputAt(0));
+              } else if (truncation.IsUsedAsFloat64()) {
+                VisitUnop(node, UseInfo::TruncatingFloat64(),
+                          MachineRepresentation::kFloat64);
+                if (lower()) DeferReplacement(node, node->InputAt(0));
+              } else {
+                VisitUnop(
+                    node,
+                    UseInfo(MachineRepresentation::kFloat64, Truncation::Any()),
+                    MachineRepresentation::kFloat64, Type::Number());
+              }
+              break;
+            case CheckFloat64HoleMode::kNeverReturnHole:
+              VisitUnop(
+                  node,
+                  UseInfo(MachineRepresentation::kFloat64, Truncation::Any()),
+                  MachineRepresentation::kFloat64, Type::Number());
+              break;
+          }
         }
         return;
       }

diff --git a/src/compiler/typed-optimization.cc b/src/compiler/typed-optimization.cc
@@ -78,6 +78,8 @@ Reduction TypedOptimization::Reduce(Node* node) {
       return ReduceCheckHeapObject(node);
     case IrOpcode::kCheckMaps:
       return ReduceCheckMaps(node);
+    case IrOpcode::kCheckNumber:
+      return ReduceCheckNumber(node);
     case IrOpcode::kCheckString:
       return ReduceCheckString(node);
     case IrOpcode::kLoadField:
@@ -152,6 +154,16 @@ Reduction TypedOptimization::ReduceCheckMaps(Node* node) {
   return NoChange();
 }
 
+Reduction TypedOptimization::ReduceCheckNumber(Node* node) {
+  Node* const input = NodeProperties::GetValueInput(node, 0);
+  Type* const input_type = NodeProperties::GetType(input);
+  if (input_type->Is(Type::Number())) {
+    ReplaceWithValue(node, input);
+    return Replace(input);
+  }
+  return NoChange();
+}
+
 Reduction TypedOptimization::ReduceCheckString(Node* node) {
   Node* const input = NodeProperties::GetValueInput(node, 0);
   Type* const input_type = NodeProperties::GetType(input);

diff --git a/src/compiler/typed-optimization.h b/src/compiler/typed-optimization.h
@@ -44,6 +44,7 @@ class V8_EXPORT_PRIVATE TypedOptimization final
  private:
   Reduction ReduceCheckHeapObject(Node* node);
   Reduction ReduceCheckMaps(Node* node);
+  Reduction ReduceCheckNumber(Node* node);
   Reduction ReduceCheckString(Node* node);
   Reduction ReduceLoadField(Node* node);
   Reduction ReduceNumberFloor(Node* node);

diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc
@@ -1839,8 +1839,7 @@ Type* Typer::Visitor::TypeCheckMaps(Node* node) {
 }
 
 Type* Typer::Visitor::TypeCheckNumber(Node* node) {
-  Type* arg = Operand(node, 0);
-  return Type::Intersect(arg, Type::Number(), zone());
+  return typer_->operation_typer_.CheckNumber(Operand(node, 0));
 }
 
 Type* Typer::Visitor::TypeCheckReceiver(Node* node) {
@@ -1859,8 +1858,7 @@ Type* Typer::Visitor::TypeCheckString(Node* node) {
 }
 
 Type* Typer::Visitor::TypeCheckFloat64Hole(Node* node) {
-  Type* type = Operand(node, 0);
-  return type;
+  return typer_->operation_typer_.CheckFloat64Hole(Operand(node, 0));
 }
 
 Type* Typer::Visitor::TypeCheckTaggedHole(Node* node) {

diff --git a/src/compiler/types.h b/src/compiler/types.h
@@ -156,6 +156,7 @@ namespace compiler {
   V(NullOrNumber,                 kNull | kNumber) \
   V(NullOrUndefined,              kNull | kUndefined) \
   V(Undetectable,                 kNullOrUndefined | kOtherUndetectable) \
+  V(NumberOrHole,                 kNumber | kHole) \
   V(NumberOrOddball,              kNumber | kNullOrUndefined | kBoolean | \
                                   kHole) \
   V(NumberOrString,               kNumber | kString) \

diff --git a/src/compiler/verifier.cc b/src/compiler/verifier.cc
@@ -1201,8 +1201,8 @@ void Verifier::Visitor::Check(Node* node) {
       break;
 
     case IrOpcode::kCheckFloat64Hole:
-      CheckValueInputIs(node, 0, Type::Number());
-      CheckTypeIs(node, Type::Number());
+      CheckValueInputIs(node, 0, Type::NumberOrHole());
+      CheckTypeIs(node, Type::NumberOrUndefined());
       break;
     case IrOpcode::kCheckTaggedHole:
       CheckValueInputIs(node, 0, Type::Any());

diff --git a/src/deoptimizer.cc b/src/deoptimizer.cc
@@ -3071,10 +3071,6 @@ void TranslatedValue::MaterializeSimple() {
     }
 
     case kDouble: {
-      if (double_value().is_hole_nan()) {
-        value_ = isolate()->factory()->hole_nan_value();
-        return;
-      }
       double scalar_value = double_value().get_scalar();
       value_ = Handle<Object>(isolate()->factory()->NewNumber(scalar_value));
       return;
@@ -4113,10 +4109,10 @@ Handle<Object> TranslatedState::MaterializeCapturedObjectAt(
             Handle<FixedDoubleArray>::cast(object);
         for (int i = 0; i < length; ++i) {
           Handle<Object> value = materializer.FieldAt(value_index);
-          CHECK(value->IsNumber());
-          if (value.is_identical_to(isolate_->factory()->hole_nan_value())) {
+          if (value.is_identical_to(isolate_->factory()->the_hole_value())) {
             double_array->set_the_hole(isolate_, i);
           } else {
+            CHECK(value->IsNumber());
             double_array->set(i, value->Number());
           }
         }

diff --git a/src/objects-inl.h b/src/objects-inl.h
@@ -1971,6 +1971,10 @@ void Oddball::set_to_number_raw(double value) {
   WRITE_DOUBLE_FIELD(this, kToNumberRawOffset, value);
 }
 
+void Oddball::set_to_number_raw_as_bits(uint64_t bits) {
+  WRITE_UINT64_FIELD(this, kToNumberRawOffset, bits);
+}
+
 ACCESSORS(Oddball, to_string, String, kToStringOffset)
 ACCESSORS(Oddball, to_number, Object, kToNumberOffset)
 ACCESSORS(Oddball, type_of, String, kTypeOfOffset)

diff --git a/src/objects.cc b/src/objects.cc
@@ -13123,7 +13123,12 @@ void Oddball::Initialize(Isolate* isolate, Handle<Oddball> oddball,
       isolate->factory()->InternalizeUtf8String(to_string);
   Handle<String> internalized_type_of =
       isolate->factory()->InternalizeUtf8String(type_of);
-  oddball->set_to_number_raw(to_number->Number());
+  if (to_number->IsHeapNumber()) {
+    oddball->set_to_number_raw_as_bits(
+        Handle<HeapNumber>::cast(to_number)->value_as_bits());
+  } else {
+    oddball->set_to_number_raw(to_number->Number());
+  }
   oddball->set_to_number(*to_number);
   oddball->set_to_string(*internalized_to_string);
   oddball->set_type_of(*internalized_type_of);

diff --git a/src/objects.h b/src/objects.h
@@ -8839,6 +8839,7 @@ class Oddball: public HeapObject {
   // [to_number_raw]: Cached raw to_number computed at startup.
   inline double to_number_raw() const;
   inline void set_to_number_raw(double value);
+  inline void set_to_number_raw_as_bits(uint64_t bits);
 
   // [to_string]: Cached to_string computed at startup.
   DECL_ACCESSORS(to_string, String)

diff --git a/test/mjsunit/regress/regress-crbug-709753.js b/test/mjsunit/regress/regress-crbug-709753.js
@@ -0,0 +1,13 @@
+// Copyright 2017 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 foo(a, i) { a[i].x; }
+
+var a = [,0.1];
+foo(a, 1);
+foo(a, 1);
+%OptimizeFunctionOnNextCall(foo);
+assertThrows(() => foo(a, 0), TypeError);