Merged: [map] Try to in-place transition during map update
When searching for a target map during map update, attempt to
update field representations in-place to the more general
representation, where possible.
Bug: chromium:1143772
No-Try: true
No-Presubmit: true
No-Tree-Checks: true
TBR=leszeks@chromium.org, fgm@chromium.org
(cherry picked from commit 8e3ae62d294818733a0322d8e8abd53d4e410f19)
Change-Id: I659890c2f08c14d1cf94242fb875c19837df2dbb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2509599
Reviewed-by: Francis McCabe <fgm@chromium.org>
Reviewed-by: Michael Hablich <hablich@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/branch-heads/8.6@{#44}
Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}
diff --git a/src/objects/map-updater.cc b/src/objects/map-updater.cc
index e51bcfc..5033318 100644
--- a/src/objects/map-updater.cc
+++ b/src/objects/map-updater.cc
@@ -401,7 +401,17 @@
}
Representation tmp_representation = tmp_details.representation();
if (!old_details.representation().fits_into(tmp_representation)) {
- break;
+ // Try updating the field in-place to a generalized type.
+ Representation generalized =
+ tmp_representation.generalize(old_details.representation());
+ if (!tmp_representation.CanBeInPlaceChangedTo(generalized)) {
+ break;
+ }
+ Handle<Map> field_owner(tmp_map->FindFieldOwner(isolate_, i), isolate_);
+ tmp_representation = generalized;
+ GeneralizeField(field_owner, i, tmp_details.constness(),
+ tmp_representation,
+ handle(tmp_descriptors->GetFieldType(i), isolate_));
}
if (tmp_details.location() == kField) {
diff --git a/src/objects/map.cc b/src/objects/map.cc
index 5f31081..0464485 100644
--- a/src/objects/map.cc
+++ b/src/objects/map.cc
@@ -613,6 +613,7 @@
transitions.GetTarget(i).DeprecateTransitionTree(isolate);
}
DCHECK(!constructor_or_backpointer().IsFunctionTemplateInfo());
+ DCHECK(CanBeDeprecated());
set_is_deprecated(true);
if (FLAG_trace_maps) {
LOG(isolate, MapEvent("Deprecate", handle(*this, isolate), Handle<Map>()));
diff --git a/test/cctest/test-field-type-tracking.cc b/test/cctest/test-field-type-tracking.cc
index a9fa589..2f59d7b 100644
--- a/test/cctest/test-field-type-tracking.cc
+++ b/test/cctest/test-field-type-tracking.cc
@@ -1038,7 +1038,8 @@
// where "p2A" and "p2B" differ only in the attributes.
//
void TestReconfigureDataFieldAttribute_GeneralizeField(
- const CRFTData& from, const CRFTData& to, const CRFTData& expected) {
+ const CRFTData& from, const CRFTData& to, const CRFTData& expected,
+ bool expected_deprecation) {
Isolate* isolate = CcTest::i_isolate();
Expectations expectations(isolate);
@@ -1107,24 +1108,29 @@
CHECK_NE(*map2, *new_map);
CHECK(expectations2.Check(*map2));
- // |map| should be deprecated and |new_map| should match new expectations.
for (int i = kSplitProp; i < kPropCount; i++) {
expectations.SetDataField(i, expected.constness, expected.representation,
expected.type);
}
- CHECK(map->is_deprecated());
- CHECK(!code_field_type->marked_for_deoptimization());
- CHECK(!code_field_repr->marked_for_deoptimization());
- CHECK(!code_field_const->marked_for_deoptimization());
- CHECK_NE(*map, *new_map);
+ if (expected_deprecation) {
+ // |map| should be deprecated and |new_map| should match new expectations.
+ CHECK(map->is_deprecated());
+ CHECK(!code_field_type->marked_for_deoptimization());
+ CHECK(!code_field_repr->marked_for_deoptimization());
+ CHECK(!code_field_const->marked_for_deoptimization());
+ CHECK_NE(*map, *new_map);
- CHECK(!new_map->is_deprecated());
- CHECK(expectations.Check(*new_map));
+ CHECK(!new_map->is_deprecated());
+ CHECK(expectations.Check(*new_map));
- // Update deprecated |map|, it should become |new_map|.
- Handle<Map> updated_map = Map::Update(isolate, map);
- CHECK_EQ(*new_map, *updated_map);
- CheckMigrationTarget(isolate, *map, *updated_map);
+ // Update deprecated |map|, it should become |new_map|.
+ Handle<Map> updated_map = Map::Update(isolate, map);
+ CHECK_EQ(*new_map, *updated_map);
+ CheckMigrationTarget(isolate, *map, *updated_map);
+ } else {
+ CHECK(!map->is_deprecated());
+ CHECK(expectations.Check(*map));
+ }
}
// This test ensures that trivial field generalization (from HeapObject to
@@ -1240,22 +1246,22 @@
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kConst, Representation::Smi(), any_type},
{PropertyConstness::kConst, Representation::Double(), any_type},
- {PropertyConstness::kConst, Representation::Double(), any_type});
+ {PropertyConstness::kConst, Representation::Double(), any_type}, true);
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kConst, Representation::Smi(), any_type},
{PropertyConstness::kMutable, Representation::Double(), any_type},
- {PropertyConstness::kMutable, Representation::Double(), any_type});
+ {PropertyConstness::kMutable, Representation::Double(), any_type}, true);
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kMutable, Representation::Smi(), any_type},
{PropertyConstness::kConst, Representation::Double(), any_type},
- {PropertyConstness::kMutable, Representation::Double(), any_type});
+ {PropertyConstness::kMutable, Representation::Double(), any_type}, true);
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kMutable, Representation::Smi(), any_type},
{PropertyConstness::kMutable, Representation::Double(), any_type},
- {PropertyConstness::kMutable, Representation::Double(), any_type});
+ {PropertyConstness::kMutable, Representation::Double(), any_type}, true);
}
TEST(ReconfigureDataFieldAttribute_GeneralizeSmiFieldToTagged) {
@@ -1270,22 +1276,26 @@
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kConst, Representation::Smi(), any_type},
{PropertyConstness::kConst, Representation::HeapObject(), value_type},
- {PropertyConstness::kConst, Representation::Tagged(), any_type});
+ {PropertyConstness::kConst, Representation::Tagged(), any_type},
+ !FLAG_modify_field_representation_inplace);
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kConst, Representation::Smi(), any_type},
{PropertyConstness::kMutable, Representation::HeapObject(), value_type},
- {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+ {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+ !FLAG_modify_field_representation_inplace);
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kMutable, Representation::Smi(), any_type},
{PropertyConstness::kConst, Representation::HeapObject(), value_type},
- {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+ {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+ !FLAG_modify_field_representation_inplace);
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kMutable, Representation::Smi(), any_type},
{PropertyConstness::kMutable, Representation::HeapObject(), value_type},
- {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+ {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+ !FLAG_modify_field_representation_inplace);
}
TEST(ReconfigureDataFieldAttribute_GeneralizeDoubleFieldToTagged) {
@@ -1300,22 +1310,26 @@
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kConst, Representation::Double(), any_type},
{PropertyConstness::kConst, Representation::HeapObject(), value_type},
- {PropertyConstness::kConst, Representation::Tagged(), any_type});
+ {PropertyConstness::kConst, Representation::Tagged(), any_type},
+ FLAG_unbox_double_fields || !FLAG_modify_field_representation_inplace);
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kConst, 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);
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kMutable, Representation::Double(), any_type},
{PropertyConstness::kConst, 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);
TestReconfigureDataFieldAttribute_GeneralizeField(
{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);
}
TEST(ReconfigureDataFieldAttribute_GeneralizeHeapObjFieldToHeapObj) {
@@ -1401,7 +1415,8 @@
TestReconfigureDataFieldAttribute_GeneralizeField(
{PropertyConstness::kMutable, Representation::HeapObject(), value_type},
{PropertyConstness::kMutable, Representation::Smi(), any_type},
- {PropertyConstness::kMutable, Representation::Tagged(), any_type});
+ {PropertyConstness::kMutable, Representation::Tagged(), any_type},
+ !FLAG_modify_field_representation_inplace);
}
// Checks that given |map| is deprecated and that it updates to given |new_map|
diff --git a/test/mjsunit/regress/regress-1143772.js b/test/mjsunit/regress/regress-1143772.js
new file mode 100644
index 0000000..40bc494
--- /dev/null
+++ b/test/mjsunit/regress/regress-1143772.js
@@ -0,0 +1,71 @@
+// Copyright 2020 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() {
+ // Only run this test if doubles are transitioned in-place to tagged.
+ let x = {};
+ x.a = 0.1;
+ let y = {};
+ y.a = {};
+ if (!%HaveSameMap(x, y)) return;
+
+ // m1: {}
+ let m1 = {};
+
+ // m2: {a:d}
+ let m2 = {};
+ assertTrue(%HaveSameMap(m2, m1));
+ m2.a = 13.37;
+
+ // m3: {a:d, b:s}
+ let m3 = {};
+ m3.a = 13.37;
+ assertTrue(%HaveSameMap(m3, m2));
+ m3.b = 1;
+
+ // m4: {a:d, b:s, c:h}
+ let m4 = {};
+ m4.a = 13.37;
+ m4.b = 1;
+ assertTrue(%HaveSameMap(m4, m3));
+ m4.c = {};
+
+ // m4_2 == m4
+ let m4_2 = {};
+ m4_2.a = 13.37;
+ m4_2.b = 1;
+ m4_2.c = {};
+ assertTrue(%HaveSameMap(m4_2, m4));
+
+ // m5: {a:d, b:d}
+ let m5 = {};
+ m5.a = 13.37;
+ assertTrue(%HaveSameMap(m5, m2));
+ m5.b = 13.37;
+ assertFalse(%HaveSameMap(m5, m3));
+
+ // At this point, Map3 and Map4 are both deprecated. Map2 transitions to
+ // Map5. Map5 is the migration target for Map3.
+ assertFalse(%HaveSameMap(m5, m3));
+
+ // m6: {a:d, b:d, c:d}
+ let m6 = {};
+ m6.a = 13.37;
+ assertTrue(%HaveSameMap(m6, m2));
+ m6.b = 13.37;
+ assertTrue(%HaveSameMap(m6, m5));
+ m6.c = 13.37
+
+ // Make m7: {a:d, b:d, c:t}
+ let m7 = m4_2;
+ assertTrue(%HaveSameMap(m7, m4));
+ // Map4 is deprecated, so this property access triggers a Map migration.
+ // With in-place map updates and no double unboxing, this should end up
+ // migrating to Map6, and updating it in-place.
+ m7.c;
+ assertFalse(%HaveSameMap(m7, m4));
+ assertTrue(%HaveSameMap(m6, m7));
+})();
