Commit 60d3f89d authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[turbofan] Consistently use the StringLength operator.

Previously all internal accesses to the String::length field in TurboFan
would use the StringLength operator, whereas explicit `string.length`
accesses from user JavaScript code would use LoadField operators instead.
This inconsistency led to redundant loads of the String::length, for
example in case of code like

```js
subject.substring(1, subject.length - 1)
```

where the `subject.substring` call introduces a StringLength(subject)
node, and the `subject.length` introduces a LoadField[length](subject)
node.

Consistently using StringLength operator everywhere enables
optimizations in TurboFan that had been blocked before here (besides
avoiding the redundant load operations).

Bug: v8:8015
Change-Id: I21c82bc418105b9933a9e60dde11c7b222dfcf4f
Reviewed-on: https://chromium-review.googlesource.com/1212942
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55710}
parent b3a480a6
......@@ -74,7 +74,7 @@ ElementAccessInfo::ElementAccessInfo(MapHandles const& receiver_maps,
// static
PropertyAccessInfo PropertyAccessInfo::NotFound(MapHandles const& receiver_maps,
MaybeHandle<JSObject> holder) {
return PropertyAccessInfo(holder, receiver_maps);
return PropertyAccessInfo(kNotFound, holder, receiver_maps);
}
// static
......@@ -111,14 +111,21 @@ PropertyAccessInfo PropertyAccessInfo::ModuleExport(
receiver_maps);
}
// static
PropertyAccessInfo PropertyAccessInfo::StringLength(
MapHandles const& receiver_maps) {
return PropertyAccessInfo(kStringLength, MaybeHandle<JSObject>(),
receiver_maps);
}
PropertyAccessInfo::PropertyAccessInfo()
: kind_(kInvalid),
field_representation_(MachineRepresentation::kNone),
field_type_(Type::None()) {}
PropertyAccessInfo::PropertyAccessInfo(MaybeHandle<JSObject> holder,
PropertyAccessInfo::PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
MapHandles const& receiver_maps)
: kind_(kNotFound),
: kind_(kind),
receiver_maps_(receiver_maps),
holder_(holder),
field_representation_(MachineRepresentation::kNone),
......@@ -218,7 +225,8 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that,
return false;
}
case kNotFound: {
case kNotFound:
case kStringLength: {
this->receiver_maps_.insert(this->receiver_maps_.end(),
that->receiver_maps_.begin(),
that->receiver_maps_.end());
......@@ -620,18 +628,20 @@ bool AccessInfoFactory::ConsolidateElementLoad(MapHandles const& maps,
bool AccessInfoFactory::LookupSpecialFieldAccessor(
Handle<Map> map, Handle<Name> name, PropertyAccessInfo* access_info) {
// Check for String::length field accessor.
if (map->IsStringMap()) {
if (Name::Equals(isolate(), name, factory()->length_string())) {
*access_info = PropertyAccessInfo::StringLength(MapHandles{map});
return true;
}
return false;
}
// Check for special JSObject field accessors.
FieldIndex field_index;
if (Accessors::IsJSObjectFieldAccessor(isolate(), map, name, &field_index)) {
Type field_type = Type::NonInternal();
MachineRepresentation field_representation = MachineRepresentation::kTagged;
if (map->IsStringMap()) {
DCHECK(Name::Equals(isolate(), factory()->length_string(), name));
// The String::length property is always a smi in the range
// [0, String::kMaxLength].
field_type = type_cache_.kStringLengthType;
field_representation = MachineRepresentation::kTaggedSigned;
} else if (map->IsJSArrayMap()) {
if (map->IsJSArrayMap()) {
DCHECK(Name::Equals(isolate(), factory()->length_string(), name));
// The JSArray::length property is a smi in the range
// [0, FixedDoubleArray::kMaxLength] in case of fast double
......
......@@ -65,7 +65,8 @@ class PropertyAccessInfo final {
kDataField,
kDataConstantField,
kAccessorConstant,
kModuleExport
kModuleExport,
kStringLength
};
static PropertyAccessInfo NotFound(MapHandles const& receiver_maps,
......@@ -84,6 +85,7 @@ class PropertyAccessInfo final {
MaybeHandle<JSObject> holder);
static PropertyAccessInfo ModuleExport(MapHandles const& receiver_maps,
Handle<Cell> cell);
static PropertyAccessInfo StringLength(MapHandles const& receiver_maps);
PropertyAccessInfo();
......@@ -98,6 +100,7 @@ class PropertyAccessInfo final {
bool IsDataConstantField() const { return kind() == kDataConstantField; }
bool IsAccessorConstant() const { return kind() == kAccessorConstant; }
bool IsModuleExport() const { return kind() == kModuleExport; }
bool IsStringLength() const { return kind() == kStringLength; }
bool HasTransitionMap() const { return !transition_map().is_null(); }
......@@ -115,7 +118,7 @@ class PropertyAccessInfo final {
Handle<Cell> export_cell() const;
private:
PropertyAccessInfo(MaybeHandle<JSObject> holder,
PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
MapHandles const& receiver_maps);
PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
Handle<Object> constant, MapHandles const& receiver_maps);
......
......@@ -1001,6 +1001,16 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
this_effect = graph()->NewNode(simplified()->MapGuard(maps), receiver,
this_effect, this_control);
}
// If all {receiver_maps} are Strings we also need to rename the
// {receiver} here to make sure that TurboFan knows that along this
// path the {this_receiver} is a String. This is because we want
// strict checking of types, for example for StringLength operators.
if (HasOnlyStringMaps(receiver_maps)) {
this_receiver = this_effect =
graph()->NewNode(common()->TypeGuard(Type::String()), receiver,
this_effect, this_control);
}
}
// Generate the actual property access.
......@@ -1837,6 +1847,8 @@ JSNativeContextSpecialization::BuildPropertyLoad(
value = effect =
graph()->NewNode(simplified()->LoadField(AccessBuilder::ForCellValue()),
cell, effect, control);
} else if (access_info.IsStringLength()) {
value = graph()->NewNode(simplified()->StringLength(), receiver);
} else {
DCHECK(access_info.IsDataField() || access_info.IsDataConstantField());
value = access_builder.BuildLoadDataField(name, access_info, receiver,
......
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