[compiler] Fix typing JSLoadNamed of private brands
Private method loads are compiled to a named load of a private brand, which always loads a BlockContext. This BlockContext holds the private methods common to all instances of a class. TurboFan currently considers JSLoadNamed to be of Type::NonInternal(). Private methods break this assumption, since BlockContext is of Type::OtherInternal(). This CL changes the typing of JSLoadNamed of private brands to be Type::OtherInternal(). Bug: v8:12500 Change-Id: I91f39747bf9422bd419d299f44152f567d8be8db Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3351167 Reviewed-by: Maya Lekova <mslekova@chromium.org> Commit-Queue: Shu-yu Guo <syg@chromium.org> Cr-Commit-Position: refs/heads/main@{#78431}
This commit is contained in:
parent
6c30d63ab9
commit
d19a707d14
@ -431,8 +431,9 @@ bool AccessInfoFactory::ComputeElementAccessInfos(
|
||||
}
|
||||
|
||||
PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
|
||||
MapRef receiver_map, MapRef map, base::Optional<JSObjectRef> holder,
|
||||
InternalIndex descriptor, AccessMode access_mode) const {
|
||||
MapRef receiver_map, MapRef map, NameRef name,
|
||||
base::Optional<JSObjectRef> holder, InternalIndex descriptor,
|
||||
AccessMode access_mode) const {
|
||||
DCHECK(descriptor.is_found());
|
||||
// TODO(jgruber,v8:7790): Use DescriptorArrayRef instead.
|
||||
Handle<DescriptorArray> descriptors = map.instance_descriptors().object();
|
||||
@ -449,7 +450,10 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
|
||||
}
|
||||
FieldIndex field_index = FieldIndex::ForPropertyIndex(*map.object(), index,
|
||||
details_representation);
|
||||
Type field_type = Type::NonInternal();
|
||||
// Private brands are used when loading private methods, which are stored in a
|
||||
// BlockContext, an internal object.
|
||||
Type field_type = name.object()->IsPrivateBrand() ? Type::OtherInternal()
|
||||
: Type::NonInternal();
|
||||
base::Optional<MapRef> field_map;
|
||||
|
||||
ZoneVector<CompilationDependency const*> unrecorded_dependencies(zone());
|
||||
@ -842,8 +846,8 @@ PropertyAccessInfo AccessInfoFactory::ComputePropertyAccessInfo(
|
||||
}
|
||||
if (details.location() == PropertyLocation::kField) {
|
||||
if (details.kind() == PropertyKind::kData) {
|
||||
return ComputeDataFieldAccessInfo(receiver_map, map, holder, index,
|
||||
access_mode);
|
||||
return ComputeDataFieldAccessInfo(receiver_map, map, name, holder,
|
||||
index, access_mode);
|
||||
} else {
|
||||
DCHECK_EQ(PropertyKind::kAccessor, details.kind());
|
||||
// TODO(turbofan): Add support for general accessors?
|
||||
|
@ -289,8 +289,9 @@ class AccessInfoFactory final {
|
||||
base::Optional<JSObjectRef> holder,
|
||||
PropertyAttributes attrs) const;
|
||||
PropertyAccessInfo ComputeDataFieldAccessInfo(
|
||||
MapRef receiver_map, MapRef map, base::Optional<JSObjectRef> holder,
|
||||
InternalIndex descriptor, AccessMode access_mode) const;
|
||||
MapRef receiver_map, MapRef map, NameRef name,
|
||||
base::Optional<JSObjectRef> holder, InternalIndex descriptor,
|
||||
AccessMode access_mode) const;
|
||||
PropertyAccessInfo ComputeAccessorDescriptorAccessInfo(
|
||||
MapRef receiver_map, NameRef name, MapRef map,
|
||||
base::Optional<JSObjectRef> holder, InternalIndex descriptor,
|
||||
|
@ -1323,7 +1323,18 @@ Type Typer::Visitor::TypeJSGetTemplateObject(Node* node) {
|
||||
|
||||
Type Typer::Visitor::TypeJSLoadProperty(Node* node) { return Type::Any(); }
|
||||
|
||||
Type Typer::Visitor::TypeJSLoadNamed(Node* node) { return Type::NonInternal(); }
|
||||
Type Typer::Visitor::TypeJSLoadNamed(Node* node) {
|
||||
#ifdef DEBUG
|
||||
// Loading of private methods is compiled to a named load of a BlockContext
|
||||
// via a private brand, which is an internal object. However, native context
|
||||
// specialization should always apply for those cases, so assert that the name
|
||||
// is not a private brand here. Otherwise Type::NonInternal() is wrong.
|
||||
JSLoadNamedNode n(node);
|
||||
NamedAccess const& p = n.Parameters();
|
||||
DCHECK(!p.name(typer_->broker()).object()->IsPrivateBrand());
|
||||
#endif
|
||||
return Type::NonInternal();
|
||||
}
|
||||
|
||||
Type Typer::Visitor::TypeJSLoadNamedFromSuper(Node* node) {
|
||||
return Type::NonInternal();
|
||||
|
22
test/mjsunit/compiler/inline-private-method.js
Normal file
22
test/mjsunit/compiler/inline-private-method.js
Normal file
@ -0,0 +1,22 @@
|
||||
// Copyright 2021 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
|
||||
|
||||
class A {
|
||||
a() { this.#b() }
|
||||
#b() {}
|
||||
}
|
||||
|
||||
function InlinePrivateMethod() {
|
||||
for (let i = 0; i < 10; i++) {
|
||||
new A().a();
|
||||
}
|
||||
}
|
||||
|
||||
%PrepareFunctionForOptimization(A);
|
||||
%PrepareFunctionForOptimization(InlinePrivateMethod);
|
||||
InlinePrivateMethod();
|
||||
%OptimizeFunctionOnNextCall(InlinePrivateMethod);
|
||||
InlinePrivateMethod();
|
Loading…
Reference in New Issue
Block a user