[turbofan] Fix CanTreatHoleAsUndefined check.

The test for CanTreatHoleAsUndefined on keyed element access was
checking for stability of Object.prototype and Array.prototype and
even adding stability dependencies on both, which is too restrictive
and leads to unnecessary deoptimizations (and might disable further
optimization of the keyed access depending on the state of the
prototype objects during optimization). This was not intended and
is considered a (performance) bug.

Instead use the correct approach of checking whether the receiver's
prototype is one of the current Object.prototype or Array.prototype
objects (since the Array protector works isolate-wide), and then
check the Array protector and install an appropriate code dependency
on the protector only.

Bug: v8:6607
Change-Id: I0bcfe32813ca3693e7b22de31b03edb3509d0a27
Reviewed-on: https://chromium-review.googlesource.com/574849
Reviewed-by: Daniel Clifford <danno@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46743}
This commit is contained in:
Benedikt Meurer 2017-07-17 19:41:22 +02:00 committed by Commit Bot
parent a8176a530c
commit e1e35df329
3 changed files with 50 additions and 23 deletions

View File

@ -2315,33 +2315,22 @@ Node* JSNativeContextSpecialization::BuildExtendPropertiesBackingStore(
bool JSNativeContextSpecialization::CanTreatHoleAsUndefined(
MapHandles const& receiver_maps) {
// Check if the array prototype chain is intact.
if (!isolate()->IsFastArrayConstructorPrototypeChainIntact()) return false;
// Make sure both the initial Array and Object prototypes are stable.
Handle<JSObject> initial_array_prototype(
native_context()->initial_array_prototype(), isolate());
Handle<JSObject> initial_object_prototype(
native_context()->initial_object_prototype(), isolate());
if (!initial_array_prototype->map()->is_stable() ||
!initial_object_prototype->map()->is_stable()) {
return false;
}
// Check if all {receiver_maps} either have the initial Array.prototype
// or the initial Object.prototype as their prototype, as those are
// guarded by the array protector cell.
for (Handle<Map> map : receiver_maps) {
if (map->prototype() != *initial_array_prototype &&
map->prototype() != *initial_object_prototype) {
// Check if all {receiver_maps} either have one of the initial Array.prototype
// or Object.prototype objects as their prototype (in any of the current
// native contexts, as the global Array protector works isolate-wide).
for (Handle<Map> receiver_map : receiver_maps) {
DisallowHeapAllocation no_gc;
Object* const receiver_prototype = receiver_map->prototype();
if (!isolate()->IsInAnyContext(receiver_prototype,
Context::INITIAL_ARRAY_PROTOTYPE_INDEX) &&
!isolate()->IsInAnyContext(receiver_prototype,
Context::INITIAL_OBJECT_PROTOTYPE_INDEX)) {
return false;
}
}
// Install code dependencies on the prototype maps.
for (Handle<Map> map : receiver_maps) {
dependencies()->AssumePrototypeMapsStable(map, initial_object_prototype);
}
// Check if the array prototype chain is intact.
if (!isolate()->IsFastArrayConstructorPrototypeChainIntact()) return false;
// Install code dependency on the array protector cell.
dependencies()->AssumePropertyCell(factory()->array_protector());

View File

@ -0,0 +1,19 @@
// Copyright 2017 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 --opt
function get(a, i) {
return a[i];
}
get([1,,3], 0);
get([1,,3], 2);
%OptimizeFunctionOnNextCall(get);
get([1,,3], 0);
assertOptimized(get);
// This unrelated change to the Array.prototype should be fine.
Array.prototype.unrelated = 1;
assertOptimized(get);

View File

@ -0,0 +1,19 @@
// Copyright 2017 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 --opt
function get(a, i) {
return a[i];
}
get([1,,3], 0);
get([1,,3], 2);
%OptimizeFunctionOnNextCall(get);
get([1,,3], 0);
assertOptimized(get);
// This unrelated change to the Object.prototype should be fine.
Object.prototype.unrelated = 1;
assertOptimized(get);