[ic] In-place Double -> Tagged transitions

With no more MutableHeapNumber, we can make Double -> Tagged transitions
in-place, at the cost of an extra map check when accessing double fields
to make sure they are still doubles.

Bug: v8:9606
Change-Id: I74ff39ed6fba62ee223cd37dfe761f7d73020e1c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1743973Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63374}

src/compiler/access-info.cc

@@ -351,6 +351,11 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
                                                                   descriptor));
   } else if (details_representation.IsDouble()) {
     field_type = type_cache_->kFloat64;
+    if (!FLAG_unbox_double_fields) {
+      unrecorded_dependencies.push_back(
+          dependencies()->FieldRepresentationDependencyOffTheRecord(
+              map_ref, descriptor));
+    }
   } else if (details_representation.IsHeapObject()) {
     // Extract the field type from the property details (make sure its
     // representation is TaggedPointer to reflect the heap object case).
@@ -788,6 +793,12 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
             transition_map_ref, number));
   } else if (details_representation.IsDouble()) {
     field_type = type_cache_->kFloat64;
+    if (!FLAG_unbox_double_fields) {
+      transition_map_ref.SerializeOwnDescriptor(number);
+      unrecorded_dependencies.push_back(
+          dependencies()->FieldRepresentationDependencyOffTheRecord(
+              transition_map_ref, number));
+    }
   } else if (details_representation.IsHeapObject()) {
     // Extract the field type from the property details (make sure its
     // representation is TaggedPointer to reflect the heap object case).

src/ic/accessor-assembler.cc

@@ -238,7 +238,7 @@ void AccessorAssembler::HandleLoadAccessor(
 
 void AccessorAssembler::HandleLoadField(Node* holder, Node* handler_word,
                                         Variable* var_double_value,
-                                        Label* rebox_double,
+                                        Label* rebox_double, Label* miss,
                                         ExitPoint* exit_point) {
   Comment("field_load");
   Node* index = DecodeWord<LoadHandler::FieldIndexBits>(handler_word);
@@ -260,6 +260,11 @@ void AccessorAssembler::HandleLoadField(Node* holder, Node* handler_word,
           LoadObjectField(holder, offset, MachineType::Float64()));
     } else {
       Node* heap_number = LoadObjectField(holder, offset);
+      // This is not an "old" Smi value from before a Smi->Double transition.
+      // Rather, it's possible that since the last update of this IC, the Double
+      // field transitioned to a Tagged field, and was then assigned a Smi.
+      GotoIf(TaggedIsSmi(heap_number), miss);
+      GotoIfNot(IsHeapNumber(heap_number), miss);
       var_double_value->Bind(LoadHeapNumberValue(heap_number));
     }
     Goto(rebox_double);
@@ -460,7 +465,7 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
          &module_export, &interceptor);
 
   BIND(&field);
-  HandleLoadField(holder, handler_word, var_double_value, rebox_double,
+  HandleLoadField(holder, handler_word, var_double_value, rebox_double, miss,
                   exit_point);
 
   BIND(&nonexistent);

src/ic/accessor-assembler.h

@@ -275,7 +275,7 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
 
   void HandleLoadField(Node* holder, Node* handler_word,
                        Variable* var_double_value, Label* rebox_double,
-                       ExitPoint* exit_point);
+                       Label* miss, ExitPoint* exit_point);
 
   void EmitAccessCheck(TNode<Context> expected_native_context,
                        TNode<Context> context, Node* receiver,

src/objects/map.cc

@@ -787,7 +787,8 @@ void Map::GeneralizeField(Isolate* isolate, Handle<Map> map, int modify_index,
     map->PrintGeneralization(
         isolate, stdout, "field type generalization", modify_index,
         map->NumberOfOwnDescriptors(), map->NumberOfOwnDescriptors(), false,
-        details.representation(), details.representation(), old_constness,
+        details.representation(),
+        descriptors->GetDetails(modify_index).representation(), old_constness,
         new_constness, old_field_type, MaybeHandle<Object>(), new_field_type,
         MaybeHandle<Object>());
   }

src/objects/property-details.h

@@ -112,7 +112,9 @@ class Representation {
     // smi and tagged values. Doubles, however, would require a box allocation.
     if (IsNone()) return !other.IsDouble();
     if (!FLAG_modify_field_representation_inplace) return false;
-    return (IsSmi() || IsHeapObject()) && other.IsTagged();
+    return (IsSmi() || (!FLAG_unbox_double_fields && IsDouble()) ||
+            IsHeapObject()) &&
+           other.IsTagged();
   }
 
   bool is_more_general_than(const Representation& other) const {

test/cctest/test-field-type-tracking.cc

@@ -641,7 +641,7 @@ void TestGeneralizeField(int detach_property_at_index, int property_index,
                                       from.representation, from.type);
     } else {
       map = expectations.AddDataField(map, NONE, PropertyConstness::kConst,
-                                      Representation::Double(), any_type);
+                                      Representation::Smi(), any_type);
       if (i == detach_property_at_index) {
         detach_point_map = map;
       }
@@ -654,10 +654,10 @@ void TestGeneralizeField(int detach_property_at_index, int property_index,
   if (is_detached_map) {
     detach_point_map = Map::ReconfigureProperty(
         isolate, detach_point_map, detach_property_at_index, kData, NONE,
-        Representation::Tagged(), any_type);
+        Representation::Double(), any_type);
     expectations.SetDataField(detach_property_at_index,
                               PropertyConstness::kConst,
-                              Representation::Tagged(), any_type);
+                              Representation::Double(), any_type);
     CHECK(map->is_deprecated());
     CHECK(expectations.Check(*detach_point_map,
                              detach_point_map->NumberOfOwnDescriptors()));
@@ -814,7 +814,9 @@ TEST(GeneralizeDoubleFieldToTagged) {
   TestGeneralizeField(
       {PropertyConstness::kMutable, Representation::Double(), any_type},
       {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
-      {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+      {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+      FLAG_unbox_double_fields || !FLAG_modify_field_representation_inplace,
+      !FLAG_unbox_double_fields && FLAG_modify_field_representation_inplace);
 }
 
 TEST(GeneralizeHeapObjectFieldToTagged) {
@@ -2295,7 +2297,7 @@ TEST(ElementsKindTransitionFromMapOwningDescriptor) {
         {PropertyConstness::kMutable, Representation::Double(), any_type},
         {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
         {PropertyConstness::kMutable, Representation::Tagged(), any_type},
-        true);
+        FLAG_unbox_double_fields || !FLAG_modify_field_representation_inplace);
   }
 }
 
@@ -2361,7 +2363,7 @@ TEST(ElementsKindTransitionFromMapNotOwningDescriptor) {
         {PropertyConstness::kMutable, Representation::Double(), any_type},
         {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
         {PropertyConstness::kMutable, Representation::Tagged(), any_type},
-        true);
+        FLAG_unbox_double_fields || !FLAG_modify_field_representation_inplace);
   }
 }
 
@@ -2404,10 +2406,10 @@ TEST(PrototypeTransitionFromMapOwningDescriptor) {
   TestGeneralizeFieldWithSpecialTransition(
       config, {PropertyConstness::kMutable, Representation::Double(), any_type},
       {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
-      {PropertyConstness::kMutable, Representation::Tagged(), any_type}, true);
+      {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+      FLAG_unbox_double_fields || !FLAG_modify_field_representation_inplace);
 }
 
-
 TEST(PrototypeTransitionFromMapNotOwningDescriptor) {
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());
@@ -2458,10 +2460,10 @@ TEST(PrototypeTransitionFromMapNotOwningDescriptor) {
   TestGeneralizeFieldWithSpecialTransition(
       config, {PropertyConstness::kMutable, Representation::Double(), any_type},
       {PropertyConstness::kMutable, Representation::HeapObject(), value_type},
-      {PropertyConstness::kMutable, Representation::Tagged(), any_type}, true);
+      {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+      FLAG_unbox_double_fields || !FLAG_modify_field_representation_inplace);
 }
 
-
 ////////////////////////////////////////////////////////////////////////////////
 // A set of tests for higher level transitioning mechanics.
 //