Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick 146bd99e762b from v8 #26400

Merged
merged 2 commits into from Nov 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/v8/.patches
Expand Up @@ -15,3 +15,4 @@ cherry-pick-7e5c7b5964.patch
cherry-pick-6a4cd97d6691.patch
cherry-pick-6aa1e71fbd09.patch
perf_make_getpositioninfoslow_faster.patch
cherry-pick-146bd99e762b.patch
294 changes: 294 additions & 0 deletions patches/v8/cherry-pick-146bd99e762b.patch
@@ -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));
+})();