From 1c7bdc7f6f4d9512f4982590bd949f265ee9c8c3 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Wed, 15 Jun 2016 22:24:33 -0700 Subject: [PATCH] [turbofan] Properly handle dictionary maps in the prototype chain. Dictionary prototypes don't have stable maps, but still don't matter for element access. Generalized the JSNativeContextSpecialization a bit to handle everything that Crankshaft can handle in this regard. R=jarin@chromium.org BUG=chromium:616709 Review-Url: https://codereview.chromium.org/2067423003 Cr-Commit-Position: refs/heads/master@{#37019} --- src/compilation-dependencies.cc | 14 ------ src/compilation-dependencies.h | 3 -- .../js-native-context-specialization.cc | 43 ++++++++++++++----- .../js-native-context-specialization.h | 10 +++-- test/mjsunit/regress/regress-crbug-616709.js | 21 +++++++++ 5 files changed, 60 insertions(+), 31 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-616709.js diff --git a/src/compilation-dependencies.cc b/src/compilation-dependencies.cc index 96b3859e9a..183de2876e 100644 --- a/src/compilation-dependencies.cc +++ b/src/compilation-dependencies.cc @@ -124,20 +124,6 @@ void CompilationDependencies::AssumeMapStable(Handle map) { } -void CompilationDependencies::AssumePrototypeMapsStable( - Handle map, MaybeHandle prototype) { - for (PrototypeIterator i(map); !i.IsAtEnd(); i.Advance()) { - Handle const current = - PrototypeIterator::GetCurrent(i); - AssumeMapStable(handle(current->map())); - Handle last; - if (prototype.ToHandle(&last) && last.is_identical_to(current)) { - break; - } - } -} - - void CompilationDependencies::AssumeTransitionStable( Handle site) { // Do nothing if the object doesn't have any useful element transitions left. diff --git a/src/compilation-dependencies.h b/src/compilation-dependencies.h index a40eb74801..ca09ef5e11 100644 --- a/src/compilation-dependencies.h +++ b/src/compilation-dependencies.h @@ -32,9 +32,6 @@ class CompilationDependencies { Insert(DependentCode::kFieldTypeGroup, map); } void AssumeMapStable(Handle map); - void AssumePrototypeMapsStable( - Handle map, - MaybeHandle prototype = MaybeHandle()); void AssumeMapNotDeprecated(Handle map); void AssumePropertyCell(Handle cell) { Insert(DependentCode::kPropertyCellChangedGroup, cell); diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc index 50c0851612..ed351e1404 100644 --- a/src/compiler/js-native-context-specialization.cc +++ b/src/compiler/js-native-context-specialization.cc @@ -225,7 +225,8 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess( // Determine actual holder and perform prototype chain checks. Handle holder; if (access_info.holder().ToHandle(&holder)) { - AssumePrototypesStable(receiver_type, native_context, holder); + this_effect = CheckPrototypeMaps(receiver_type, native_context, holder, + this_effect, this_control); } // Generate the actual property access. @@ -668,7 +669,8 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess( // not compatible with (monomorphic) keyed stores. Handle holder; if (access_info.holder().ToHandle(&holder)) { - AssumePrototypesStable(receiver_type, native_context, holder); + this_effect = CheckPrototypeMaps(receiver_type, native_context, holder, + this_effect, this_control); } // TODO(bmeurer): We currently specialize based on elements kind. We should @@ -753,8 +755,9 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess( if (receiver_type->NowIs(initial_holey_array_type) && isolate()->IsFastArrayConstructorPrototypeChainIntact()) { // Add a code dependency on the array protector cell. - AssumePrototypesStable(receiver_type, native_context, - isolate()->initial_object_prototype()); + this_effect = CheckPrototypeMaps( + receiver_type, native_context, + isolate()->initial_object_prototype(), this_effect, this_control); dependencies()->AssumePropertyCell(factory()->array_protector()); // Turn the hole into undefined. mode = CheckTaggedHoleMode::kConvertHoleToUndefined; @@ -772,8 +775,9 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess( if (receiver_type->NowIs(initial_holey_array_type) && isolate()->IsFastArrayConstructorPrototypeChainIntact()) { // Add a code dependency on the array protector cell. - AssumePrototypesStable(receiver_type, native_context, - isolate()->initial_object_prototype()); + this_effect = CheckPrototypeMaps( + receiver_type, native_context, + isolate()->initial_object_prototype(), this_effect, this_control); dependencies()->AssumePropertyCell(factory()->array_protector()); // Return the signaling NaN hole directly if all uses are truncating. mode = CheckFloat64HoleMode::kAllowReturnHole; @@ -956,10 +960,9 @@ Reduction JSNativeContextSpecialization::ReduceJSStoreProperty(Node* node) { p.language_mode(), store_mode); } - -void JSNativeContextSpecialization::AssumePrototypesStable( +Node* JSNativeContextSpecialization::CheckPrototypeMaps( Type* receiver_type, Handle native_context, - Handle holder) { + Handle holder, Node* effect, Node* control) { // Determine actual holder and perform prototype chain checks. for (auto i = receiver_type->Classes(); !i.Done(); i.Advance()) { Handle map = i.Current(); @@ -970,8 +973,28 @@ void JSNativeContextSpecialization::AssumePrototypesStable( .ToHandle(&constructor)) { map = handle(constructor->initial_map(), isolate()); } - dependencies()->AssumePrototypeMapsStable(map, holder); + for (PrototypeIterator j(map); !j.IsAtEnd(); j.Advance()) { + Handle const current = + PrototypeIterator::GetCurrent(j); + Handle current_map(current->map(), isolate()); + if (current_map->is_stable()) { + dependencies()->AssumeMapStable(current_map); + } else { + // TODO(bmeurer): Introduce a dedicated CheckMaps operator. + Node* prototype = jsgraph()->HeapConstant(current); + Node* prototype_map = effect = + graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()), + prototype, effect, control); + Node* check = graph()->NewNode( + simplified()->ReferenceEqual(Type::Internal()), prototype_map, + jsgraph()->HeapConstant(current_map)); + effect = + graph()->NewNode(simplified()->CheckIf(), check, effect, control); + } + if (holder.is_identical_to(current)) break; + } } + return effect; } bool JSNativeContextSpecialization::ExtractReceiverMaps( diff --git a/src/compiler/js-native-context-specialization.h b/src/compiler/js-native-context-specialization.h index 7d43bfb26d..0c7d4fc086 100644 --- a/src/compiler/js-native-context-specialization.h +++ b/src/compiler/js-native-context-specialization.h @@ -80,10 +80,12 @@ class JSNativeContextSpecialization final : public AdvancedReducer { Reduction ReduceSoftDeoptimize(Node* node); // Adds stability dependencies on all prototypes of every class in - // {receiver_type} up to (and including) the {holder}. - void AssumePrototypesStable(Type* receiver_type, - Handle native_context, - Handle holder); + // {receiver_type} up to (and including) the {holder} if the maps + // are stable, otherwise falls back to inserting runtime map checks + // on the prototypes. + Node* CheckPrototypeMaps(Type* receiver_type, Handle native_context, + Handle holder, Node* effect, + Node* control); // Extract receiver maps from {nexus} and filter based on {receiver} if // possible. diff --git a/test/mjsunit/regress/regress-crbug-616709.js b/test/mjsunit/regress/regress-crbug-616709.js new file mode 100644 index 0000000000..75abe3c2e1 --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-616709.js @@ -0,0 +1,21 @@ +// Copyright 2016 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 + +// Make the Object prototype have dictionary properties. +for (var i = 0; i < 2000; i++) { + Object.prototype['X'+i] = true; +} + +function boom(a1) { + return a1[0]; +} + +var a = new Array(1); +a[0] = 0.1; +boom(a); +boom(a); +%OptimizeFunctionOnNextCall(boom); +boom(a);