Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 146bd99e762b from v8 (#26400)
- Loading branch information
Showing
2 changed files
with
295 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,294 @@ | ||
| From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
| From: "ishell@chromium.org" <ishell@chromium.org> | ||
| Date: Tue, 3 Nov 2020 12:42:43 +0100 | ||
| Subject: Merged: Squashed multiple commits. | ||
|
|
||
| Merged: [map] Try to in-place transition during map update | ||
| Revision: 8e3ae62d294818733a0322d8e8abd53d4e410f19 | ||
|
|
||
| Merged: [map] Skip loading the field owner before GeneralizeField | ||
| Revision: a928f5fcc2a67c2c8d621ece48f76deb0d36637b | ||
|
|
||
| BUG=chromium:1143772 | ||
| NOTRY=true | ||
| NOPRESUBMIT=true | ||
| NOTREECHECKS=true | ||
| R=verwaest@chromium.org | ||
|
|
||
| Change-Id: I78628cb697b21caa2098cc5948e226af5fcd020c | ||
| Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2516474 | ||
| Reviewed-by: Toon Verwaest <verwaest@chromium.org> | ||
| Cr-Commit-Position: refs/branch-heads/8.7@{#31} | ||
| Cr-Branched-From: 0d81cd72688512abcbe1601015baee390c484a6a-refs/heads/8.7.220@{#1} | ||
| Cr-Branched-From: 942c2ef85caef00fcf02517d049f05e9a3d4b440-refs/heads/master@{#70196} | ||
|
|
||
| diff --git a/src/objects/map-updater.cc b/src/objects/map-updater.cc | ||
| index 8c9b94014f8efaa4463cc883877ed52d24963d4c..5a016614a1782913e2297a4a396fc4402ea41cee 100644 | ||
| --- a/src/objects/map-updater.cc | ||
| +++ b/src/objects/map-updater.cc | ||
| @@ -232,10 +232,7 @@ MapUpdater::State MapUpdater::TryReconfigureToDataFieldInplace() { | ||
| handle(old_descriptors_->GetFieldType(modified_descriptor_), isolate_), | ||
| MaybeHandle<Object>(), new_field_type_, MaybeHandle<Object>()); | ||
| } | ||
| - Handle<Map> field_owner( | ||
| - old_map_->FindFieldOwner(isolate_, modified_descriptor_), isolate_); | ||
| - | ||
| - GeneralizeField(field_owner, modified_descriptor_, new_constness_, | ||
| + GeneralizeField(old_map_, modified_descriptor_, new_constness_, | ||
| new_representation_, new_field_type_); | ||
| // Check that the descriptor array was updated. | ||
| DCHECK(old_descriptors_->GetDetails(modified_descriptor_) | ||
| @@ -401,7 +398,13 @@ MapUpdater::State MapUpdater::FindTargetMap() { | ||
| } | ||
| 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; | ||
| + } | ||
| + tmp_representation = generalized; | ||
| } | ||
|
|
||
| if (tmp_details.location() == kField) { | ||
| diff --git a/src/objects/map.cc b/src/objects/map.cc | ||
| index 2dc288628c8ea69d425e3b86e63e151157b9f925..1799cff1a2284c9b0d7cdaec99d0c310c733042c 100644 | ||
| --- a/src/objects/map.cc | ||
| +++ b/src/objects/map.cc | ||
| @@ -609,6 +609,7 @@ void Map::DeprecateTransitionTree(Isolate* isolate) { | ||
| 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 840478a5203847f331f792eae7125a60396f896a..7abbcbde350a08bbb7d364c15184bc896607848b 100644 | ||
| --- a/test/cctest/test-field-type-tracking.cc | ||
| +++ b/test/cctest/test-field-type-tracking.cc | ||
| @@ -1019,7 +1019,8 @@ namespace { | ||
| // 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); | ||
| @@ -1079,22 +1080,26 @@ void TestReconfigureDataFieldAttribute_GeneralizeField( | ||
| 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->marked_for_deoptimization()); | ||
| - CHECK_NE(*map, *new_map); | ||
| + if (expected_deprecation) { | ||
| + CHECK(map->is_deprecated()); | ||
| + CHECK(!code->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 | ||
| @@ -1200,22 +1205,22 @@ TEST(ReconfigureDataFieldAttribute_GeneralizeSmiFieldToDouble) { | ||
| 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) { | ||
| @@ -1230,22 +1235,26 @@ TEST(ReconfigureDataFieldAttribute_GeneralizeSmiFieldToTagged) { | ||
| 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) { | ||
| @@ -1260,22 +1269,26 @@ TEST(ReconfigureDataFieldAttribute_GeneralizeDoubleFieldToTagged) { | ||
| 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) { | ||
| @@ -1361,7 +1374,8 @@ TEST(ReconfigureDataFieldAttribute_GeneralizeHeapObjectFieldToTagged) { | ||
| 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); | ||
| } | ||
|
|
||
|
|
||
| diff --git a/test/mjsunit/regress/regress-1143772.js b/test/mjsunit/regress/regress-1143772.js | ||
| new file mode 100644 | ||
| index 0000000000000000000000000000000000000000..40bc494d458afec816fd72e3fbb36b20a7942649 | ||
| --- /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)); | ||
| +})(); |