deps: V8: cherry-pick 94c87fe

Original commit message: [ic] Fix handling of +0/-0 when constant field tracking is enabled ... and ensure that runtime behaviour is in sync with the IC code. Bug: chromium:950747, v8:9113 Change-Id: Ied66c9514cbe3a4d75fc71d4fc3b19ea1538f9b2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1561319 Reviewed-by: Toon Verwaest <[email protected]> Commit-Queue: Igor Sheludko <[email protected]> Cr-Commit-Position: refs/heads/master@{#60768} PR-URL: #27792 Fixes: #27784 Refs: v8/v8@94c87fe Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
nodejs · May 22, 2019 · f2fe1e5 · f2fe1e5
1 parent c3f7251
commit f2fe1e5
Show file tree

Hide file tree

Showing 10 changed files with 328 additions and 93 deletions.
diff --git a/common.gypi b/common.gypi
@@ -38,7 +38,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.17',
+    'v8_embedder_string': '-node.18',
 
     ##### V8 defaults for Node.js #####
 

diff --git a/deps/v8/src/code-stub-assembler.cc b/deps/v8/src/code-stub-assembler.cc
@@ -12367,7 +12367,7 @@ Node* CodeStubAssembler::StrictEqual(Node* lhs, Node* rhs,
 // This algorithm differs from the Strict Equality Comparison Algorithm in its
 // treatment of signed zeroes and NaNs.
 void CodeStubAssembler::BranchIfSameValue(Node* lhs, Node* rhs, Label* if_true,
-                                          Label* if_false) {
+                                          Label* if_false, SameValueMode mode) {
   VARIABLE(var_lhs_value, MachineRepresentation::kFloat64);
   VARIABLE(var_rhs_value, MachineRepresentation::kFloat64);
   Label do_fcmp(this);
@@ -12413,10 +12413,12 @@ void CodeStubAssembler::BranchIfSameValue(Node* lhs, Node* rhs, Label* if_true,
                  if_lhsisbigint(this);
              Node* const lhs_map = LoadMap(lhs);
              GotoIf(IsHeapNumberMap(lhs_map), &if_lhsisheapnumber);
-             Node* const lhs_instance_type = LoadMapInstanceType(lhs_map);
-             GotoIf(IsStringInstanceType(lhs_instance_type), &if_lhsisstring);
-             Branch(IsBigIntInstanceType(lhs_instance_type), &if_lhsisbigint,
-                    if_false);
+             if (mode != SameValueMode::kNumbersOnly) {
+               Node* const lhs_instance_type = LoadMapInstanceType(lhs_map);
+               GotoIf(IsStringInstanceType(lhs_instance_type), &if_lhsisstring);
+               GotoIf(IsBigIntInstanceType(lhs_instance_type), &if_lhsisbigint);
+             }
+             Goto(if_false);
 
              BIND(&if_lhsisheapnumber);
              {
@@ -12426,53 +12428,62 @@ void CodeStubAssembler::BranchIfSameValue(Node* lhs, Node* rhs, Label* if_true,
                Goto(&do_fcmp);
              }
 
-             BIND(&if_lhsisstring);
-             {
-               // Now we can only yield true if {rhs} is also a String
-               // with the same sequence of characters.
-               GotoIfNot(IsString(rhs), if_false);
-               Node* const result = CallBuiltin(Builtins::kStringEqual,
-                                                NoContextConstant(), lhs, rhs);
-               Branch(IsTrue(result), if_true, if_false);
-             }
-
-             BIND(&if_lhsisbigint);
-             {
-               GotoIfNot(IsBigInt(rhs), if_false);
-               Node* const result = CallRuntime(Runtime::kBigIntEqualToBigInt,
-                                                NoContextConstant(), lhs, rhs);
-               Branch(IsTrue(result), if_true, if_false);
+             if (mode != SameValueMode::kNumbersOnly) {
+               BIND(&if_lhsisstring);
+               {
+                 // Now we can only yield true if {rhs} is also a String
+                 // with the same sequence of characters.
+                 GotoIfNot(IsString(rhs), if_false);
+                 Node* const result = CallBuiltin(
+                     Builtins::kStringEqual, NoContextConstant(), lhs, rhs);
+                 Branch(IsTrue(result), if_true, if_false);
+               }
+
+               BIND(&if_lhsisbigint);
+               {
+                 GotoIfNot(IsBigInt(rhs), if_false);
+                 Node* const result =
+                     CallRuntime(Runtime::kBigIntEqualToBigInt,
+                                 NoContextConstant(), lhs, rhs);
+                 Branch(IsTrue(result), if_true, if_false);
+               }
              }
            });
   }
 
   BIND(&do_fcmp);
   {
-    Node* const lhs_value = var_lhs_value.value();
-    Node* const rhs_value = var_rhs_value.value();
+    TNode<Float64T> lhs_value = UncheckedCast<Float64T>(var_lhs_value.value());
+    TNode<Float64T> rhs_value = UncheckedCast<Float64T>(var_rhs_value.value());
+    BranchIfSameNumberValue(lhs_value, rhs_value, if_true, if_false);
+  }
+}
 
-    Label if_equal(this), if_notequal(this);
-    Branch(Float64Equal(lhs_value, rhs_value), &if_equal, &if_notequal);
+void CodeStubAssembler::BranchIfSameNumberValue(TNode<Float64T> lhs_value,
+                                                TNode<Float64T> rhs_value,
+                                                Label* if_true,
+                                                Label* if_false) {
+  Label if_equal(this), if_notequal(this);
+  Branch(Float64Equal(lhs_value, rhs_value), &if_equal, &if_notequal);
 
-    BIND(&if_equal);
-    {
-      // We still need to handle the case when {lhs} and {rhs} are -0.0 and
-      // 0.0 (or vice versa). Compare the high word to
-      // distinguish between the two.
-      Node* const lhs_hi_word = Float64ExtractHighWord32(lhs_value);
-      Node* const rhs_hi_word = Float64ExtractHighWord32(rhs_value);
-
-      // If x is +0 and y is -0, return false.
-      // If x is -0 and y is +0, return false.
-      Branch(Word32Equal(lhs_hi_word, rhs_hi_word), if_true, if_false);
-    }
+  BIND(&if_equal);
+  {
+    // We still need to handle the case when {lhs} and {rhs} are -0.0 and
+    // 0.0 (or vice versa). Compare the high word to
+    // distinguish between the two.
+    Node* const lhs_hi_word = Float64ExtractHighWord32(lhs_value);
+    Node* const rhs_hi_word = Float64ExtractHighWord32(rhs_value);
 
-    BIND(&if_notequal);
-    {
-      // Return true iff both {rhs} and {lhs} are NaN.
-      GotoIf(Float64Equal(lhs_value, lhs_value), if_false);
-      Branch(Float64Equal(rhs_value, rhs_value), if_false, if_true);
-    }
+    // If x is +0 and y is -0, return false.
+    // If x is -0 and y is +0, return false.
+    Branch(Word32Equal(lhs_hi_word, rhs_hi_word), if_true, if_false);
+  }
+
+  BIND(&if_notequal);
+  {
+    // Return true iff both {rhs} and {lhs} are NaN.
+    GotoIf(Float64Equal(lhs_value, lhs_value), if_false);
+    Branch(Float64Equal(rhs_value, rhs_value), if_false, if_true);
   }
 }
 

diff --git a/deps/v8/src/code-stub-assembler.h b/deps/v8/src/code-stub-assembler.h
@@ -3095,7 +3095,14 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   // ECMA#sec-samevalue
   // Similar to StrictEqual except that NaNs are treated as equal and minus zero
   // differs from positive zero.
-  void BranchIfSameValue(Node* lhs, Node* rhs, Label* if_true, Label* if_false);
+  enum class SameValueMode { kNumbersOnly, kFull };
+  void BranchIfSameValue(Node* lhs, Node* rhs, Label* if_true, Label* if_false,
+                         SameValueMode mode = SameValueMode::kFull);
+  // A part of BranchIfSameValue() that handles two double values.
+  // Treats NaN == NaN and +0 != -0.
+  void BranchIfSameNumberValue(TNode<Float64T> lhs_value,
+                               TNode<Float64T> rhs_value, Label* if_true,
+                               Label* if_false);
 
   enum HasPropertyLookupMode { kHasProperty, kForInHasProperty };
 

diff --git a/deps/v8/src/ic/accessor-assembler.cc b/deps/v8/src/ic/accessor-assembler.cc
@@ -1202,23 +1202,23 @@ void AccessorAssembler::OverwriteExistingFastDataProperty(
 
     BIND(&inobject);
     {
-      Node* field_offset = TimesTaggedSize(field_index);
+      TNode<IntPtrT> field_offset = Signed(TimesTaggedSize(field_index));
       Label tagged_rep(this), double_rep(this);
       Branch(
           Word32Equal(representation, Int32Constant(Representation::kDouble)),
           &double_rep, &tagged_rep);
       BIND(&double_rep);
       {
-        Node* double_value = ChangeNumberToFloat64(value);
+        TNode<Float64T> double_value = ChangeNumberToFloat64(value);
         if (FLAG_unbox_double_fields) {
           if (do_transitioning_store) {
             StoreMap(object, object_map);
           } else if (FLAG_track_constant_fields) {
             Label if_mutable(this);
             GotoIfNot(IsPropertyDetailsConst(details), &if_mutable);
-            Node* current_value =
-                LoadObjectField(object, field_offset, MachineType::Float64());
-            Branch(Float64Equal(current_value, double_value), &done, slow);
+            TNode<Float64T> current_value =
+                LoadObjectField<Float64T>(CAST(object), field_offset);
+            BranchIfSameNumberValue(current_value, double_value, &done, slow);
             BIND(&if_mutable);
           }
           StoreObjectFieldNoWriteBarrier(object, field_offset, double_value,
@@ -1234,8 +1234,9 @@ void AccessorAssembler::OverwriteExistingFastDataProperty(
             if (FLAG_track_constant_fields) {
               Label if_mutable(this);
               GotoIfNot(IsPropertyDetailsConst(details), &if_mutable);
-              Node* current_value = LoadHeapNumberValue(mutable_heap_number);
-              Branch(Float64Equal(current_value, double_value), &done, slow);
+              TNode<Float64T> current_value =
+                  LoadHeapNumberValue(mutable_heap_number);
+              BranchIfSameNumberValue(current_value, double_value, &done, slow);
               BIND(&if_mutable);
             }
             StoreHeapNumberValue(mutable_heap_number, double_value);
@@ -1251,9 +1252,10 @@ void AccessorAssembler::OverwriteExistingFastDataProperty(
         } else if (FLAG_track_constant_fields) {
           Label if_mutable(this);
           GotoIfNot(IsPropertyDetailsConst(details), &if_mutable);
-          Node* current_value =
-              LoadObjectField(object, field_offset, MachineType::AnyTagged());
-          Branch(WordEqual(current_value, value), &done, slow);
+          TNode<Object> current_value =
+              LoadObjectField(CAST(object), field_offset);
+          BranchIfSameValue(current_value, value, &done, slow,
+                            SameValueMode::kNumbersOnly);
           BIND(&if_mutable);
         }
         StoreObjectField(object, field_offset, value);
@@ -1303,12 +1305,13 @@ void AccessorAssembler::OverwriteExistingFastDataProperty(
         {
           Node* mutable_heap_number =
               LoadPropertyArrayElement(properties, backing_store_index);
-          Node* double_value = ChangeNumberToFloat64(value);
+          TNode<Float64T> double_value = ChangeNumberToFloat64(value);
           if (FLAG_track_constant_fields) {
             Label if_mutable(this);
             GotoIfNot(IsPropertyDetailsConst(details), &if_mutable);
-            Node* current_value = LoadHeapNumberValue(mutable_heap_number);
-            Branch(Float64Equal(current_value, double_value), &done, slow);
+            TNode<Float64T> current_value =
+                LoadHeapNumberValue(mutable_heap_number);
+            BranchIfSameNumberValue(current_value, double_value, &done, slow);
             BIND(&if_mutable);
           }
           StoreHeapNumberValue(mutable_heap_number, double_value);
@@ -1319,9 +1322,10 @@ void AccessorAssembler::OverwriteExistingFastDataProperty(
           if (FLAG_track_constant_fields) {
             Label if_mutable(this);
             GotoIfNot(IsPropertyDetailsConst(details), &if_mutable);
-            Node* current_value =
+            TNode<Object> current_value =
                 LoadPropertyArrayElement(properties, backing_store_index);
-            Branch(WordEqual(current_value, value), &done, slow);
+            BranchIfSameValue(current_value, value, &done, slow,
+                              SameValueMode::kNumbersOnly);
             BIND(&if_mutable);
           }
           StorePropertyArrayElement(properties, backing_store_index, value);
@@ -1813,7 +1817,7 @@ void AccessorAssembler::StoreNamedField(Node* handler_word, Node* object,
   }
 
   Node* index = DecodeWord<StoreHandler::FieldIndexBits>(handler_word);
-  Node* offset = IntPtrMul(index, IntPtrConstant(kTaggedSize));
+  TNode<IntPtrT> offset = Signed(TimesTaggedSize(index));
   if (representation.IsDouble()) {
     if (!FLAG_unbox_double_fields || !is_inobject) {
       // Load the mutable heap number.
@@ -1831,14 +1835,14 @@ void AccessorAssembler::StoreNamedField(Node* handler_word, Node* object,
               &done);
     {
       if (store_value_as_double) {
-        Node* current_value =
-            LoadObjectField(property_storage, offset, MachineType::Float64());
-        GotoIfNot(Float64Equal(current_value, value), bailout);
+        TNode<Float64T> current_value =
+            LoadObjectField<Float64T>(CAST(property_storage), offset);
+        BranchIfSameNumberValue(current_value, UncheckedCast<Float64T>(value),
+                                &done, bailout);
       } else {
         Node* current_value = LoadObjectField(property_storage, offset);
-        GotoIfNot(WordEqual(current_value, value), bailout);
+        Branch(WordEqual(current_value, value), &done, bailout);
       }
-      Goto(&done);
     }
     BIND(&done);
   }

diff --git a/deps/v8/src/lookup.cc b/deps/v8/src/lookup.cc
@@ -934,10 +934,14 @@ bool LookupIterator::IsConstFieldValueEqualTo(Object value) const {
       // Uninitialized double field.
       return true;
     }
-    return bit_cast<double>(bits) == value->Number();
+    return Object::SameNumberValue(bit_cast<double>(bits), value->Number());
   } else {
     Object current_value = holder->RawFastPropertyAt(field_index);
-    return current_value->IsUninitialized(isolate()) || current_value == value;
+    if (current_value->IsUninitialized(isolate()) || current_value == value) {
+      return true;
+    }
+    return current_value->IsNumber() && value->IsNumber() &&
+           Object::SameNumberValue(current_value->Number(), value->Number());
   }
 }
 

diff --git a/deps/v8/src/objects-inl.h b/deps/v8/src/objects-inl.h
@@ -383,6 +383,16 @@ double Object::Number() const {
                  : HeapNumber::unchecked_cast(*this)->value();
 }
 
+// static
+bool Object::SameNumberValue(double value1, double value2) {
+  // SameNumberValue(NaN, NaN) is true.
+  if (value1 != value2) {
+    return std::isnan(value1) && std::isnan(value2);
+  }
+  // SameNumberValue(0.0, -0.0) is false.
+  return (std::signbit(value1) == std::signbit(value2));
+}
+
 bool Object::IsNaN() const {
   return this->IsHeapNumber() && std::isnan(HeapNumber::cast(*this)->value());
 }