Skip to content

Commit

Permalink
deps: V8: cherry-pick 3ba21a17ce2f
Browse files Browse the repository at this point in the history
Original commit message:

    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

    [email protected], [email protected]
    (cherry picked from commit 8e3ae62d294818733a0322d8e8abd53d4e410f19)

    Change-Id: I659890c2f08c14d1cf94242fb875c19837df2dbb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2509599
    Reviewed-by: Francis McCabe <[email protected]>
    Reviewed-by: Michael Hablich <[email protected]>
    Reviewed-by: Bill Budge <[email protected]>
    Reviewed-by: Igor Sheludko <[email protected]>
    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}

Refs: v8/v8@3ba21a1

PR-URL: #38275
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
  • Loading branch information
targos committed Apr 30, 2021
1 parent 70f622b commit 18a4cbf
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 28 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,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.37',
'v8_embedder_string': '-node.38',

##### V8 defaults for Node.js #####

Expand Down
12 changes: 11 additions & 1 deletion deps/v8/src/objects/map-updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,17 @@ 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;
}
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) {
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/objects/map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,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>()));
Expand Down
67 changes: 41 additions & 26 deletions deps/v8/test/cctest/test-field-type-tracking.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,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);
Expand Down Expand Up @@ -1121,24 +1122,29 @@ 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_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
Expand Down Expand Up @@ -1254,22 +1260,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) {
Expand All @@ -1284,22 +1290,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) {
Expand All @@ -1314,22 +1324,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) {
Expand Down Expand Up @@ -1415,7 +1429,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);
}

// Checks that given |map| is deprecated and that it updates to given |new_map|
Expand Down
71 changes: 71 additions & 0 deletions deps/v8/test/mjsunit/regress/regress-1143772.js
Original file line number Diff line number Diff line change
@@ -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));
})();

0 comments on commit 18a4cbf

Please sign in to comment.