Commit 4cc4bc59 authored by ishell's avatar ishell Committed by Commit bot

Map::TryUpdate() must be in sync with Map::Update().

This CL fixes elements kind transitions handling in Map::TryUpdate().

BUG=v8:4121
LOG=Y

Review URL: https://codereview.chromium.org/1181163002

Cr-Commit-Position: refs/heads/master@{#28999}
parent 103fcfaa
......@@ -2420,7 +2420,7 @@ Handle<Map> Map::ReconfigureProperty(Handle<Map> old_map, int modify_index,
ElementsKind from_kind = root_map->elements_kind();
ElementsKind to_kind = old_map->elements_kind();
if (from_kind != to_kind &&
if (from_kind != to_kind && to_kind != DICTIONARY_ELEMENTS &&
!(IsTransitionableFastElementsKind(from_kind) &&
IsMoreGeneralElementsKindTransition(from_kind, to_kind))) {
return CopyGeneralizeAllRepresentations(old_map, modify_index, store_mode,
......@@ -2884,6 +2884,15 @@ MaybeHandle<Map> Map::TryUpdate(Handle<Map> old_map) {
// Check the state of the root map.
Map* root_map = old_map->FindRootMap();
if (!old_map->EquivalentToForTransition(root_map)) return MaybeHandle<Map>();
ElementsKind from_kind = root_map->elements_kind();
ElementsKind to_kind = old_map->elements_kind();
if (from_kind != to_kind) {
// Try to follow existing elements kind transitions.
root_map = root_map->LookupElementsTransitionMap(to_kind);
if (root_map == NULL) return MaybeHandle<Map>();
// From here on, use the map with correct elements kind as root map.
}
int root_nof = root_map->NumberOfOwnDescriptors();
int old_nof = old_map->NumberOfOwnDescriptors();
......
......@@ -20,11 +20,6 @@
using namespace v8::internal;
// TODO(ishell): fix this once ReconfigureProperty supports "non equivalent"
// transitions.
const bool IS_NON_EQUIVALENT_TRANSITION_SUPPORTED = false;
// TODO(ishell): fix this once TransitionToPrototype stops generalizing
// all field representations (similar to crbug/448711 where elements kind
// and observed transitions caused generalization of all field representations).
......@@ -1591,7 +1586,14 @@ static void TestGeneralizeRepresentationWithSpecialTransition(
CHECK(!new_map2->is_deprecated());
CHECK(!new_map2->is_dictionary_map());
if (!IS_NON_EQUIVALENT_TRANSITION_SUPPORTED) {
Handle<Map> tmp_map;
if (Map::TryUpdate(map2).ToHandle(&tmp_map)) {
// If Map::TryUpdate() manages to succeed the result must match the result
// of Map::Update().
CHECK_EQ(*new_map2, *tmp_map);
}
if (config.is_non_equevalent_transition()) {
// In case of non-equivalent transition currently we generalize all
// representations.
for (int i = 0; i < kPropCount; i++) {
......@@ -1600,7 +1602,8 @@ static void TestGeneralizeRepresentationWithSpecialTransition(
CHECK(new_map2->GetBackPointer()->IsUndefined());
CHECK(expectations2.Check(*new_map2));
} else {
CHECK(expectations.Check(*new_map2));
CHECK(!new_map2->GetBackPointer()->IsUndefined());
CHECK(expectations2.Check(*new_map2));
}
}
......@@ -1632,6 +1635,7 @@ TEST(ElementsKindTransitionFromMapOwningDescriptor) {
}
// TODO(ishell): remove once IS_PROTO_TRANS_ISSUE_FIXED is removed.
bool generalizes_representations() const { return false; }
bool is_non_equevalent_transition() const { return false; }
};
TestConfig config;
TestGeneralizeRepresentationWithSpecialTransition(
......@@ -1666,6 +1670,7 @@ TEST(ElementsKindTransitionFromMapNotOwningDescriptor) {
}
// TODO(ishell): remove once IS_PROTO_TRANS_ISSUE_FIXED is removed.
bool generalizes_representations() const { return false; }
bool is_non_equevalent_transition() const { return false; }
};
TestConfig config;
TestGeneralizeRepresentationWithSpecialTransition(
......@@ -1688,6 +1693,7 @@ TEST(ForObservedTransitionFromMapOwningDescriptor) {
}
// TODO(ishell): remove once IS_PROTO_TRANS_ISSUE_FIXED is removed.
bool generalizes_representations() const { return false; }
bool is_non_equevalent_transition() const { return true; }
};
TestConfig config;
TestGeneralizeRepresentationWithSpecialTransition(
......@@ -1721,6 +1727,7 @@ TEST(ForObservedTransitionFromMapNotOwningDescriptor) {
}
// TODO(ishell): remove once IS_PROTO_TRANS_ISSUE_FIXED is removed.
bool generalizes_representations() const { return false; }
bool is_non_equevalent_transition() const { return true; }
};
TestConfig config;
TestGeneralizeRepresentationWithSpecialTransition(
......@@ -1754,6 +1761,7 @@ TEST(PrototypeTransitionFromMapOwningDescriptor) {
bool generalizes_representations() const {
return !IS_PROTO_TRANS_ISSUE_FIXED;
}
bool is_non_equevalent_transition() const { return true; }
};
TestConfig config;
TestGeneralizeRepresentationWithSpecialTransition(
......@@ -1798,6 +1806,7 @@ TEST(PrototypeTransitionFromMapNotOwningDescriptor) {
bool generalizes_representations() const {
return !IS_PROTO_TRANS_ISSUE_FIXED;
}
bool is_non_equevalent_transition() const { return true; }
};
TestConfig config;
TestGeneralizeRepresentationWithSpecialTransition(
......
// Copyright 2015 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 Migrator(o) {
return o.foo;
}
function Loader(o) {
return o[0];
}
var first_smi_array = [1];
var second_smi_array = [2];
var first_object_array = ["first"];
var second_object_array = ["string"];
assertTrue(%HasFastSmiElements(first_smi_array));
assertTrue(%HasFastSmiElements(second_smi_array));
assertTrue(%HasFastObjectElements(first_object_array));
assertTrue(%HasFastObjectElements(second_object_array));
// Prepare identical transition chains for smi and object arrays.
first_smi_array.foo = 0;
second_smi_array.foo = 0;
first_object_array.foo = 0;
second_object_array.foo = 0;
// Collect type feedback for not-yet-deprecated original object array map.
for (var i = 0; i < 3; i++) Migrator(second_object_array);
// Blaze a migration trail for smi array maps.
// This marks the migrated smi array map as a migration target.
first_smi_array.foo = 0.5;
print(second_smi_array.foo);
// Deprecate original object array map.
// Use TryMigrate from deferred optimized code to migrate second object array.
first_object_array.foo = 0.5;
%OptimizeFunctionOnNextCall(Migrator);
Migrator(second_object_array);
// |second_object_array| now erroneously has a smi map.
// Optimized code assuming smi elements will expose this.
for (var i = 0; i < 3; i++) Loader(second_smi_array);
%OptimizeFunctionOnNextCall(Loader);
assertEquals("string", Loader(second_object_array));
// Any of the following checks will also fail:
assertTrue(%HasFastObjectElements(second_object_array));
assertFalse(%HasFastSmiElements(second_object_array));
assertTrue(%HaveSameMap(first_object_array, second_object_array));
assertFalse(%HaveSameMap(first_smi_array, second_object_array));
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment