Commit b1883dc3 authored by Frank Emrich's avatar Frank Emrich Committed by Commit Bot

[dict-proto] TF support for constants in dictionary mode protos, pt. 3

This CL is part of a  series that implements Turbofan support for
property accesses satisfying the following conditions:
1. The holder is a dictionary mode object.
2. The holder is a prototype.
3. The access is a load.

This feature will only be enabled if the build flag
v8_dict_property_const_tracking is set.

This particular CL implements support for the case that the property
in question is an accesor, meaning that the given PropertyAccessInfo
has kind kAccessorDictionaryProtoConstant.

Bug: v8:11248
Change-Id: Id082107edd45fa91a3f1d96aa9df345a60f46917
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2720300Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Commit-Queue: Frank Emrich <emrich@google.com>
Cr-Commit-Position: refs/heads/master@{#73607}
parent 2858fcf7
......@@ -135,7 +135,7 @@ PropertyAccessInfo PropertyAccessInfo::FastAccessorConstant(
Zone* zone, Handle<Map> receiver_map, Handle<Object> constant,
MaybeHandle<JSObject> holder) {
return PropertyAccessInfo(zone, kFastAccessorConstant, holder, constant,
{{receiver_map}, zone});
MaybeHandle<Name>(), {{receiver_map}, zone});
}
// static
......@@ -143,7 +143,7 @@ PropertyAccessInfo PropertyAccessInfo::ModuleExport(Zone* zone,
Handle<Map> receiver_map,
Handle<Cell> cell) {
return PropertyAccessInfo(zone, kModuleExport, MaybeHandle<JSObject>(), cell,
{{receiver_map}, zone});
MaybeHandle<Name>{}, {{receiver_map}, zone});
}
// static
......@@ -164,9 +164,9 @@ PropertyAccessInfo PropertyAccessInfo::DictionaryProtoDataConstant(
// static
PropertyAccessInfo PropertyAccessInfo::DictionaryProtoAccessorConstant(
Zone* zone, Handle<Map> receiver_map, MaybeHandle<JSObject> holder,
Handle<Object> constant) {
Handle<Object> constant, Handle<Name> property_name) {
return PropertyAccessInfo(zone, kDictionaryProtoAccessorConstant, holder,
constant, {{receiver_map}, zone});
constant, property_name, {{receiver_map}, zone});
}
// static
......@@ -204,7 +204,8 @@ PropertyAccessInfo::PropertyAccessInfo(
PropertyAccessInfo::PropertyAccessInfo(
Zone* zone, Kind kind, MaybeHandle<JSObject> holder,
Handle<Object> constant, ZoneVector<Handle<Map>>&& lookup_start_object_maps)
Handle<Object> constant, MaybeHandle<Name> property_name,
ZoneVector<Handle<Map>>&& lookup_start_object_maps)
: kind_(kind),
lookup_start_object_maps_(lookup_start_object_maps),
constant_(constant),
......@@ -212,7 +213,11 @@ PropertyAccessInfo::PropertyAccessInfo(
unrecorded_dependencies_(zone),
field_representation_(Representation::None()),
field_type_(Type::Any()),
dictionary_index_(InternalIndex::NotFound()) {}
dictionary_index_(InternalIndex::NotFound()),
name_(property_name) {
DCHECK_IMPLIES(kind == kDictionaryProtoAccessorConstant,
!property_name.is_null());
}
PropertyAccessInfo::PropertyAccessInfo(
Kind kind, MaybeHandle<JSObject> holder, MaybeHandle<Map> transition_map,
FieldIndex field_index, Representation field_representation,
......@@ -320,6 +325,7 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that,
return false;
}
case kDictionaryProtoAccessorConstant:
case kFastAccessorConstant: {
// Check if we actually access the same constant.
if (this->constant_.address() == that->constant_.address()) {
......@@ -358,10 +364,6 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that,
}
case kModuleExport:
return false;
case kDictionaryProtoAccessorConstant:
// TODO(v8:11248) Dealt with in follow-up CLs.
UNREACHABLE();
}
}
......@@ -515,54 +517,57 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
UNREACHABLE();
}
PropertyAccessInfo AccessInfoFactory::ComputeAccessorDescriptorAccessInfo(
Handle<Map> receiver_map, Handle<Name> name, Handle<Map> map,
MaybeHandle<JSObject> holder, InternalIndex descriptor,
AccessMode access_mode) const {
DCHECK(descriptor.is_found());
Handle<DescriptorArray> descriptors(map->instance_descriptors(isolate()),
isolate());
SLOW_DCHECK(descriptor == descriptors->Search(*name, *map));
namespace {
using AccessorsObjectGetter = std::function<Handle<Object>()>;
PropertyAccessInfo AccessorAccessInfoHelper(
Isolate* isolate, Zone* zone, JSHeapBroker* broker,
const AccessInfoFactory* ai_factory, Handle<Map> receiver_map,
Handle<Name> name, Handle<Map> map, MaybeHandle<JSObject> holder,
AccessMode access_mode, AccessorsObjectGetter get_accessors) {
if (map->instance_type() == JS_MODULE_NAMESPACE_TYPE) {
DCHECK(map->is_prototype_map());
Handle<PrototypeInfo> proto_info(PrototypeInfo::cast(map->prototype_info()),
isolate());
isolate);
Handle<JSModuleNamespace> module_namespace(
JSModuleNamespace::cast(proto_info->module_namespace()), isolate());
JSModuleNamespace::cast(proto_info->module_namespace()), isolate);
Handle<Cell> cell(Cell::cast(module_namespace->module().exports().Lookup(
isolate(), name, Smi::ToInt(name->GetHash()))),
isolate());
if (cell->value().IsTheHole(isolate())) {
isolate, name, Smi::ToInt(name->GetHash()))),
isolate);
if (cell->value().IsTheHole(isolate)) {
// This module has not been fully initialized yet.
return PropertyAccessInfo::Invalid(zone());
return PropertyAccessInfo::Invalid(zone);
}
return PropertyAccessInfo::ModuleExport(zone(), receiver_map, cell);
return PropertyAccessInfo::ModuleExport(zone, receiver_map, cell);
}
if (access_mode == AccessMode::kHas) {
// kHas is not supported for dictionary mode objects.
DCHECK(!map->is_dictionary_map());
// HasProperty checks don't call getter/setters, existence is sufficient.
return PropertyAccessInfo::FastAccessorConstant(zone(), receiver_map,
return PropertyAccessInfo::FastAccessorConstant(zone, receiver_map,
Handle<Object>(), holder);
}
Handle<Object> accessors(descriptors->GetStrongValue(descriptor), isolate());
Handle<Object> accessors = get_accessors();
if (!accessors->IsAccessorPair()) {
return PropertyAccessInfo::Invalid(zone());
return PropertyAccessInfo::Invalid(zone);
}
Handle<Object> accessor(access_mode == AccessMode::kLoad
? Handle<AccessorPair>::cast(accessors)->getter()
: Handle<AccessorPair>::cast(accessors)->setter(),
isolate());
isolate);
if (!accessor->IsJSFunction()) {
CallOptimization optimization(isolate(), accessor);
CallOptimization optimization(isolate, accessor);
if (!optimization.is_simple_api_call() ||
optimization.IsCrossContextLazyAccessorPair(
*broker()->target_native_context().object(), *map)) {
return PropertyAccessInfo::Invalid(zone());
*broker->target_native_context().object(), *map)) {
return PropertyAccessInfo::Invalid(zone);
}
CallOptimization::HolderLookup lookup;
holder = optimization.LookupHolderOfExpectedType(receiver_map, &lookup);
if (lookup == CallOptimization::kHolderNotFound) {
return PropertyAccessInfo::Invalid(zone());
return PropertyAccessInfo::Invalid(zone);
}
DCHECK_IMPLIES(lookup == CallOptimization::kHolderIsReceiver,
holder.is_null());
......@@ -570,15 +575,39 @@ PropertyAccessInfo AccessInfoFactory::ComputeAccessorDescriptorAccessInfo(
}
if (access_mode == AccessMode::kLoad) {
Handle<Name> cached_property_name;
if (FunctionTemplateInfo::TryGetCachedPropertyName(isolate(), accessor)
if (FunctionTemplateInfo::TryGetCachedPropertyName(isolate, accessor)
.ToHandle(&cached_property_name)) {
PropertyAccessInfo access_info =
ComputePropertyAccessInfo(map, cached_property_name, access_mode);
PropertyAccessInfo access_info = ai_factory->ComputePropertyAccessInfo(
map, cached_property_name, access_mode);
if (!access_info.IsInvalid()) return access_info;
}
}
return PropertyAccessInfo::FastAccessorConstant(zone(), receiver_map,
accessor, holder);
if (map->is_dictionary_map()) {
return PropertyAccessInfo::DictionaryProtoAccessorConstant(
zone, receiver_map, holder, accessor, name);
} else {
return PropertyAccessInfo::FastAccessorConstant(zone, receiver_map,
accessor, holder);
}
}
} // namespace
PropertyAccessInfo AccessInfoFactory::ComputeAccessorDescriptorAccessInfo(
Handle<Map> receiver_map, Handle<Name> name, Handle<Map> holder_map,
MaybeHandle<JSObject> holder, InternalIndex descriptor,
AccessMode access_mode) const {
DCHECK(descriptor.is_found());
Handle<DescriptorArray> descriptors(
holder_map->instance_descriptors(kRelaxedLoad), isolate());
SLOW_DCHECK(descriptor == descriptors->Search(*name, *holder_map));
auto get_accessors = [&]() {
return handle(descriptors->GetStrongValue(descriptor), isolate());
};
return AccessorAccessInfoHelper(isolate(), zone(), broker(), this,
receiver_map, name, holder_map, holder,
access_mode, get_accessors);
}
PropertyAccessInfo AccessInfoFactory::ComputeDictionaryProtoAccessInfo(
......@@ -599,8 +628,13 @@ PropertyAccessInfo AccessInfoFactory::ComputeDictionaryProtoAccessInfo(
zone(), receiver_map, holder, dictionary_index, name);
}
// TODO(v8:11248) Support for accessors is implemented a in follow-up CL.
return PropertyAccessInfo::Invalid(zone());
auto get_accessors = [&]() {
return JSObject::DictionaryPropertyAt(holder, dictionary_index);
};
Handle<Map> holder_map = handle(holder->map(), isolate());
return AccessorAccessInfoHelper(isolate(), zone(), broker(), this,
receiver_map, name, holder_map, holder,
access_mode, get_accessors);
}
MinimorphicLoadPropertyAccessInfo AccessInfoFactory::ComputePropertyAccessInfo(
......
......@@ -105,7 +105,7 @@ class PropertyAccessInfo final {
InternalIndex dict_index, Handle<Name> name);
static PropertyAccessInfo DictionaryProtoAccessorConstant(
Zone* zone, Handle<Map> receiver_map, MaybeHandle<JSObject> holder,
Handle<Object> constant);
Handle<Object> constant, Handle<Name> name);
bool Merge(PropertyAccessInfo const* that, AccessMode access_mode,
Zone* zone) V8_WARN_UNUSED_RESULT;
......@@ -183,7 +183,7 @@ class PropertyAccessInfo final {
PropertyAccessInfo(Zone* zone, Kind kind, MaybeHandle<JSObject> holder,
ZoneVector<Handle<Map>>&& lookup_start_object_maps);
PropertyAccessInfo(Zone* zone, Kind kind, MaybeHandle<JSObject> holder,
Handle<Object> constant,
Handle<Object> constant, MaybeHandle<Name> name,
ZoneVector<Handle<Map>>&& lookup_start_object_maps);
PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
MaybeHandle<Map> transition_map, FieldIndex field_index,
......
......@@ -110,10 +110,11 @@ class ConstantInDictionaryPrototypeChainDependency final
public:
explicit ConstantInDictionaryPrototypeChainDependency(
const MapRef receiver_map, const NameRef property_name,
const ObjectRef constant)
const ObjectRef constant, PropertyKind kind)
: receiver_map_(receiver_map),
property_name_{property_name},
constant_{constant} {
constant_{constant},
kind_{kind} {
DCHECK(V8_DICT_PROPERTY_CONST_TRACKING_BOOL);
}
......@@ -166,7 +167,26 @@ class ConstantInDictionaryPrototypeChainDependency final
return ValidationResult::kFoundIncorrect;
}
Object value = dictionary.ValueAt(entry);
Object dictionary_value = dictionary.ValueAt(entry);
Object value;
// We must be able to detect the case that the property |property_name_|
// of |holder_| was originally a plain function |constant_| (when creating
// this dependency) and has since become an accessor whose getter is
// |constant_|. Therefore, we cannot just look at the property kind of
// |details|, because that reflects the current situation, not the one
// when creating this dependency.
if (details.kind() != kind_) {
return ValidationResult::kFoundIncorrect;
}
if (kind_ == PropertyKind::kAccessor) {
// Only supporting loading at the moment, so we only ever want the
// getter.
CHECK(dictionary_value.IsAccessorPair());
value = AccessorPair::cast(dictionary_value)
.get(AccessorComponent::ACCESSOR_GETTER);
} else {
value = dictionary_value;
}
return value == *constant_.object() ? ValidationResult::kFoundCorrect
: ValidationResult::kFoundIncorrect;
};
......@@ -202,6 +222,7 @@ class ConstantInDictionaryPrototypeChainDependency final
MapRef receiver_map_;
NameRef property_name_;
ObjectRef constant_;
PropertyKind kind_;
};
class TransitionDependency final : public CompilationDependency {
......@@ -504,9 +525,9 @@ void CompilationDependencies::DependOnStableMap(const MapRef& map) {
void CompilationDependencies::DependOnConstantInDictionaryPrototypeChain(
const MapRef& receiver_map, const NameRef& property_name,
const ObjectRef& constant) {
const ObjectRef& constant, PropertyKind kind) {
RecordDependency(zone_->New<ConstantInDictionaryPrototypeChainDependency>(
receiver_map, property_name, constant));
receiver_map, property_name, constant, kind));
}
AllocationType CompilationDependencies::DependOnPretenureMode(
......
......@@ -51,9 +51,13 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
// of the objects between receiver and |holder| on the prototype chain, b) any
// of the objects on the prototype chain up to |holder| change prototypes, or
// c) the value of |property_name| in |holder| changes.
// If PropertyKind is kData, |constant| is the value of the property in
// question. In case of PropertyKind::kAccessor, |constant| is the accessor
// function (i.e., getter or setter) itself, not the overall AccessorPair.
void DependOnConstantInDictionaryPrototypeChain(const MapRef& receiver_map,
const NameRef& property_name,
const ObjectRef& constant);
const ObjectRef& constant,
PropertyKind kind);
// Return the pretenure mode of {site} and record the assumption that it does
// not change.
......
......@@ -2224,6 +2224,16 @@ Node* JSNativeContextSpecialization::InlinePropertyGetterCall(
Node* frame_state, Node** effect, Node** control,
ZoneVector<Node*>* if_exceptions, PropertyAccessInfo const& access_info) {
ObjectRef constant(broker(), access_info.constant());
if (access_info.IsDictionaryProtoAccessorConstant()) {
// For fast mode holders we recorded dependencies in BuildPropertyLoad.
for (const Handle<Map> map : access_info.lookup_start_object_maps()) {
dependencies()->DependOnConstantInDictionaryPrototypeChain(
MapRef{broker(), map}, NameRef{broker(), access_info.name()},
constant, PropertyKind::kAccessor);
}
}
Node* target = jsgraph()->Constant(constant);
// Introduce the call to the getter function.
Node* value;
......@@ -2359,7 +2369,8 @@ JSNativeContextSpecialization::BuildPropertyLoad(
Node* value;
if (access_info.IsNotFound()) {
value = jsgraph()->UndefinedConstant();
} else if (access_info.IsFastAccessorConstant()) {
} else if (access_info.IsFastAccessorConstant() ||
access_info.IsDictionaryProtoAccessorConstant()) {
ConvertReceiverMode receiver_mode =
receiver == lookup_start_object
? ConvertReceiverMode::kNotNullOrUndefined
......
......@@ -160,7 +160,7 @@ Node* PropertyAccessBuilder::FoldLoadDictPrototypeConstant(
for (const Handle<Map> map : access_info.lookup_start_object_maps()) {
dependencies()->DependOnConstantInDictionaryPrototypeChain(
MapRef{broker(), map}, NameRef{broker(), access_info.name()},
value.value());
value.value(), PropertyKind::kData);
}
return jsgraph()->Constant(value.value());
......
......@@ -3012,7 +3012,8 @@ SerializerForBackgroundCompilation::ProcessMapForNamedPropertyAccess(
// For JSNativeContextSpecialization::InlinePropertySetterCall
// and InlinePropertyGetterCall.
if (access_info.IsFastAccessorConstant() &&
if ((access_info.IsFastAccessorConstant() ||
access_info.IsDictionaryProtoAccessorConstant()) &&
!access_info.constant().is_null()) {
if (access_info.constant()->IsJSFunction()) {
JSFunctionRef function(broker(), access_info.constant());
......
......@@ -372,6 +372,38 @@ function testbench(o, proto, update_proto, check_constness) {
}
})();
// Test inlining of accessor.
(function() {
var proto = Object.create(null);
proto.x_val = 1;
Object.defineProperty(proto, "x", {
get : function () {return this.x_val;}
});
var o = Object.create(proto);
assertFalse(%HasFastProperties(proto))
function read_x(arg_o) {
return arg_o.x;
}
%PrepareFunctionForOptimization(read_x);
assertEquals(1, read_x(o));
%OptimizeFunctionOnNextCall(read_x);
assertEquals(1, read_x(o));
assertOptimized(read_x);
// Test that we inlined the access:
var dummy = {x : 123};
read_x(dummy);
if (%IsDictPropertyConstTrackingEnabled()) {
assertTrue(%HasFastProperties(o));
assertFalse(%HasFastProperties(proto));
assertUnoptimized(read_x);
}
})();
// Invalidation by adding same property to receiver.
(function() {
var proto = Object.create(null);
......
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