Commit f85826ea authored by Georg Schmid's avatar Georg Schmid Committed by Commit Bot

[turbofan] Track field owner maps during load elimination

This CL adds additional information in PropertyAccessInfos and FieldAccesses about the map that introduced the accessed field. We use this information to prevent load elimination from incorrectly optimizing certain accesses marked const.

Prior to this CL, load elimination simply stored information about eliminatable field accesses based on objects (identified by nodes in the graph) and offsets (i.e., statically known ones). In the presence of const stores and loads this is insufficient, since a single object (in the above sense) may contain distinct *const* properties at the same offset throughout its lifetime. As an example, consider the following piece of code:

    let obj = {};
    obj.a = 0;
    obj[1024] = 1;  // An offset of >=1024 forces an elements-kind transition
    delete obj.a;
    obj.b = 2;
    assertEquals(obj.b, 2);

In this scenario, *both* the first ('obj.a = 0') and the second ('obj.b = 2') store to a field will be marked const by the runtime. The reason that storing to 'a' above ends up being marked const, is that 'a' before and after the elements-kind transition is encoded in separate transition trees. Removing 'a' ('delete obj.a') only invalidates const-ness in the dictionary-elements transition tree; not the holey-elements one used at the time of 'obj.a = 0'.

The above situation on its own violates an invariant in load elimination. Namely, we assume that for the same object and offset, we will never encounter two const stores. One can extend the above snippet to coax load-elimination into producing incorrect results. For instance, by "hiding" 'obj.b = 2' in an unoptimized function call, the consecutive load from 'b' will incorrectly produce 0, violating the assert.

R=neis@chromium.org, tebbi@chromium.org

Bug: chromium:980183, chromium:983764
Change-Id: I576a9c7efd416fa9db6daff1f42d483e4bd369b4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1751346
Commit-Queue: Georg Schmid <gsps@google.com>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63226}
parent 35558d38
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "src/builtins/accessors.h" #include "src/builtins/accessors.h"
#include "src/compiler/compilation-dependencies.h" #include "src/compiler/compilation-dependencies.h"
#include "src/compiler/compilation-dependency.h" #include "src/compiler/compilation-dependency.h"
#include "src/compiler/simplified-operator.h"
#include "src/compiler/type-cache.h" #include "src/compiler/type-cache.h"
#include "src/ic/call-optimization.h" #include "src/ic/call-optimization.h"
#include "src/logging/counters.h" #include "src/logging/counters.h"
...@@ -81,11 +82,12 @@ PropertyAccessInfo PropertyAccessInfo::DataField( ...@@ -81,11 +82,12 @@ PropertyAccessInfo PropertyAccessInfo::DataField(
Zone* zone, Handle<Map> receiver_map, Zone* zone, Handle<Map> receiver_map,
ZoneVector<CompilationDependency const*>&& dependencies, ZoneVector<CompilationDependency const*>&& dependencies,
FieldIndex field_index, Representation field_representation, FieldIndex field_index, Representation field_representation,
Type field_type, MaybeHandle<Map> field_map, MaybeHandle<JSObject> holder, Type field_type, Handle<Map> field_owner_map, MaybeHandle<Map> field_map,
MaybeHandle<Map> transition_map) { MaybeHandle<JSObject> holder, MaybeHandle<Map> transition_map) {
return PropertyAccessInfo(kDataField, holder, transition_map, field_index, return PropertyAccessInfo(kDataField, holder, transition_map, field_index,
field_representation, field_type, field_map, field_representation, field_type, field_owner_map,
{{receiver_map}, zone}, std::move(dependencies)); field_map, {{receiver_map}, zone},
std::move(dependencies));
} }
// static // static
...@@ -93,11 +95,12 @@ PropertyAccessInfo PropertyAccessInfo::DataConstant( ...@@ -93,11 +95,12 @@ PropertyAccessInfo PropertyAccessInfo::DataConstant(
Zone* zone, Handle<Map> receiver_map, Zone* zone, Handle<Map> receiver_map,
ZoneVector<CompilationDependency const*>&& dependencies, ZoneVector<CompilationDependency const*>&& dependencies,
FieldIndex field_index, Representation field_representation, FieldIndex field_index, Representation field_representation,
Type field_type, MaybeHandle<Map> field_map, MaybeHandle<JSObject> holder, Type field_type, Handle<Map> field_owner_map, MaybeHandle<Map> field_map,
MaybeHandle<Map> transition_map) { MaybeHandle<JSObject> holder, MaybeHandle<Map> transition_map) {
return PropertyAccessInfo(kDataConstant, holder, transition_map, field_index, return PropertyAccessInfo(kDataConstant, holder, transition_map, field_index,
field_representation, field_type, field_map, field_representation, field_type, field_owner_map,
{{receiver_map}, zone}, std::move(dependencies)); field_map, {{receiver_map}, zone},
std::move(dependencies));
} }
// static // static
...@@ -155,7 +158,7 @@ PropertyAccessInfo::PropertyAccessInfo(Zone* zone, Kind kind, ...@@ -155,7 +158,7 @@ PropertyAccessInfo::PropertyAccessInfo(Zone* zone, Kind kind,
PropertyAccessInfo::PropertyAccessInfo( PropertyAccessInfo::PropertyAccessInfo(
Kind kind, MaybeHandle<JSObject> holder, MaybeHandle<Map> transition_map, Kind kind, MaybeHandle<JSObject> holder, MaybeHandle<Map> transition_map,
FieldIndex field_index, Representation field_representation, FieldIndex field_index, Representation field_representation,
Type field_type, MaybeHandle<Map> field_map, Type field_type, Handle<Map> field_owner_map, MaybeHandle<Map> field_map,
ZoneVector<Handle<Map>>&& receiver_maps, ZoneVector<Handle<Map>>&& receiver_maps,
ZoneVector<CompilationDependency const*>&& unrecorded_dependencies) ZoneVector<CompilationDependency const*>&& unrecorded_dependencies)
: kind_(kind), : kind_(kind),
...@@ -166,7 +169,11 @@ PropertyAccessInfo::PropertyAccessInfo( ...@@ -166,7 +169,11 @@ PropertyAccessInfo::PropertyAccessInfo(
field_index_(field_index), field_index_(field_index),
field_representation_(field_representation), field_representation_(field_representation),
field_type_(field_type), field_type_(field_type),
field_map_(field_map) {} field_owner_map_(field_owner_map),
field_map_(field_map) {
DCHECK_IMPLIES(!transition_map.is_null(),
field_owner_map.address() == transition_map.address());
}
bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that, bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that,
AccessMode access_mode, Zone* zone) { AccessMode access_mode, Zone* zone) {
...@@ -258,6 +265,13 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that, ...@@ -258,6 +265,13 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that,
} }
} }
ConstFieldInfo PropertyAccessInfo::GetConstFieldInfo() const {
if (IsDataConstant()) {
return ConstFieldInfo(field_owner_map_.ToHandleChecked());
}
return ConstFieldInfo::None();
}
AccessInfoFactory::AccessInfoFactory(JSHeapBroker* broker, AccessInfoFactory::AccessInfoFactory(JSHeapBroker* broker,
CompilationDependencies* dependencies, CompilationDependencies* dependencies,
Zone* zone) Zone* zone)
...@@ -378,15 +392,19 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo( ...@@ -378,15 +392,19 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
map_ref.SerializeOwnDescriptor(descriptor); map_ref.SerializeOwnDescriptor(descriptor);
constness = dependencies()->DependOnFieldConstness(map_ref, descriptor); constness = dependencies()->DependOnFieldConstness(map_ref, descriptor);
} }
Handle<Map> field_owner_map(map->FindFieldOwner(isolate(), descriptor),
isolate());
switch (constness) { switch (constness) {
case PropertyConstness::kMutable: case PropertyConstness::kMutable:
return PropertyAccessInfo::DataField( return PropertyAccessInfo::DataField(
zone(), receiver_map, std::move(unrecorded_dependencies), field_index, zone(), receiver_map, std::move(unrecorded_dependencies), field_index,
details_representation, field_type, field_map, holder); details_representation, field_type, field_owner_map, field_map,
holder);
case PropertyConstness::kConst: case PropertyConstness::kConst:
return PropertyAccessInfo::DataConstant( return PropertyAccessInfo::DataConstant(
zone(), receiver_map, std::move(unrecorded_dependencies), field_index, zone(), receiver_map, std::move(unrecorded_dependencies), field_index,
details_representation, field_type, field_map, holder); details_representation, field_type, field_owner_map, field_map,
holder);
} }
UNREACHABLE(); UNREACHABLE();
} }
...@@ -724,7 +742,7 @@ PropertyAccessInfo AccessInfoFactory::LookupSpecialFieldAccessor( ...@@ -724,7 +742,7 @@ PropertyAccessInfo AccessInfoFactory::LookupSpecialFieldAccessor(
} }
// Special fields are always mutable. // Special fields are always mutable.
return PropertyAccessInfo::DataField(zone(), map, {{}, zone()}, field_index, return PropertyAccessInfo::DataField(zone(), map, {{}, zone()}, field_index,
field_representation, field_type); field_representation, field_type, map);
} }
return PropertyAccessInfo::Invalid(zone()); return PropertyAccessInfo::Invalid(zone());
} }
...@@ -800,12 +818,12 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition( ...@@ -800,12 +818,12 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
case PropertyConstness::kMutable: case PropertyConstness::kMutable:
return PropertyAccessInfo::DataField( return PropertyAccessInfo::DataField(
zone(), map, std::move(unrecorded_dependencies), field_index, zone(), map, std::move(unrecorded_dependencies), field_index,
details_representation, field_type, field_map, holder, details_representation, field_type, transition_map, field_map, holder,
transition_map); transition_map);
case PropertyConstness::kConst: case PropertyConstness::kConst:
return PropertyAccessInfo::DataConstant( return PropertyAccessInfo::DataConstant(
zone(), map, std::move(unrecorded_dependencies), field_index, zone(), map, std::move(unrecorded_dependencies), field_index,
details_representation, field_type, field_map, holder, details_representation, field_type, transition_map, field_map, holder,
transition_map); transition_map);
} }
UNREACHABLE(); UNREACHABLE();
......
...@@ -29,6 +29,7 @@ class CompilationDependency; ...@@ -29,6 +29,7 @@ class CompilationDependency;
class ElementAccessFeedback; class ElementAccessFeedback;
class JSHeapBroker; class JSHeapBroker;
class TypeCache; class TypeCache;
struct ConstFieldInfo;
std::ostream& operator<<(std::ostream&, AccessMode); std::ostream& operator<<(std::ostream&, AccessMode);
...@@ -77,14 +78,16 @@ class PropertyAccessInfo final { ...@@ -77,14 +78,16 @@ class PropertyAccessInfo final {
Zone* zone, Handle<Map> receiver_map, Zone* zone, Handle<Map> receiver_map,
ZoneVector<CompilationDependency const*>&& unrecorded_dependencies, ZoneVector<CompilationDependency const*>&& unrecorded_dependencies,
FieldIndex field_index, Representation field_representation, FieldIndex field_index, Representation field_representation,
Type field_type, MaybeHandle<Map> field_map = MaybeHandle<Map>(), Type field_type, Handle<Map> field_owner_map,
MaybeHandle<Map> field_map = MaybeHandle<Map>(),
MaybeHandle<JSObject> holder = MaybeHandle<JSObject>(), MaybeHandle<JSObject> holder = MaybeHandle<JSObject>(),
MaybeHandle<Map> transition_map = MaybeHandle<Map>()); MaybeHandle<Map> transition_map = MaybeHandle<Map>());
static PropertyAccessInfo DataConstant( static PropertyAccessInfo DataConstant(
Zone* zone, Handle<Map> receiver_map, Zone* zone, Handle<Map> receiver_map,
ZoneVector<CompilationDependency const*>&& unrecorded_dependencies, ZoneVector<CompilationDependency const*>&& unrecorded_dependencies,
FieldIndex field_index, Representation field_representation, FieldIndex field_index, Representation field_representation,
Type field_type, MaybeHandle<Map> field_map, MaybeHandle<JSObject> holder, Type field_type, Handle<Map> field_owner_map, MaybeHandle<Map> field_map,
MaybeHandle<JSObject> holder,
MaybeHandle<Map> transition_map = MaybeHandle<Map>()); MaybeHandle<Map> transition_map = MaybeHandle<Map>());
static PropertyAccessInfo AccessorConstant(Zone* zone, static PropertyAccessInfo AccessorConstant(Zone* zone,
Handle<Map> receiver_map, Handle<Map> receiver_map,
...@@ -109,6 +112,7 @@ class PropertyAccessInfo final { ...@@ -109,6 +112,7 @@ class PropertyAccessInfo final {
bool IsStringLength() const { return kind() == kStringLength; } bool IsStringLength() const { return kind() == kStringLength; }
bool HasTransitionMap() const { return !transition_map().is_null(); } bool HasTransitionMap() const { return !transition_map().is_null(); }
ConstFieldInfo GetConstFieldInfo() const;
Kind kind() const { return kind_; } Kind kind() const { return kind_; }
MaybeHandle<JSObject> holder() const { MaybeHandle<JSObject> holder() const {
...@@ -137,7 +141,7 @@ class PropertyAccessInfo final { ...@@ -137,7 +141,7 @@ class PropertyAccessInfo final {
PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder, PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
MaybeHandle<Map> transition_map, FieldIndex field_index, MaybeHandle<Map> transition_map, FieldIndex field_index,
Representation field_representation, Type field_type, Representation field_representation, Type field_type,
MaybeHandle<Map> field_map, Handle<Map> field_owner_map, MaybeHandle<Map> field_map,
ZoneVector<Handle<Map>>&& receiver_maps, ZoneVector<Handle<Map>>&& receiver_maps,
ZoneVector<CompilationDependency const*>&& dependencies); ZoneVector<CompilationDependency const*>&& dependencies);
...@@ -150,6 +154,7 @@ class PropertyAccessInfo final { ...@@ -150,6 +154,7 @@ class PropertyAccessInfo final {
FieldIndex field_index_; FieldIndex field_index_;
Representation field_representation_; Representation field_representation_;
Type field_type_; Type field_type_;
MaybeHandle<Map> field_owner_map_;
MaybeHandle<Map> field_map_; MaybeHandle<Map> field_map_;
}; };
......
...@@ -1615,6 +1615,7 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control, ...@@ -1615,6 +1615,7 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control,
DCHECK_EQ(kData, property_details.kind()); DCHECK_EQ(kData, property_details.kind());
NameRef property_name = boilerplate_map.GetPropertyKey(i); NameRef property_name = boilerplate_map.GetPropertyKey(i);
FieldIndex index = boilerplate_map.GetFieldIndexFor(i); FieldIndex index = boilerplate_map.GetFieldIndexFor(i);
ConstFieldInfo const_field_info(boilerplate_map.object());
FieldAccess access = {kTaggedBase, FieldAccess access = {kTaggedBase,
index.offset(), index.offset(),
property_name.object(), property_name.object(),
...@@ -1623,7 +1624,7 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control, ...@@ -1623,7 +1624,7 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control,
MachineType::TypeCompressedTagged(), MachineType::TypeCompressedTagged(),
kFullWriteBarrier, kFullWriteBarrier,
LoadSensitivity::kUnsafe, LoadSensitivity::kUnsafe,
property_details.constness()}; const_field_info};
Node* value; Node* value;
if (boilerplate_map.IsUnboxedDoubleField(i)) { if (boilerplate_map.IsUnboxedDoubleField(i)) {
access.machine_type = MachineType::Float64(); access.machine_type = MachineType::Float64();
...@@ -1636,7 +1637,7 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control, ...@@ -1636,7 +1637,7 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control,
// the field. The hole NaN should therefore be unobservable. // the field. The hole NaN should therefore be unobservable.
// Load elimination expects there to be at most one const store to any // Load elimination expects there to be at most one const store to any
// given field, so we always mark the unobservable ones as mutable. // given field, so we always mark the unobservable ones as mutable.
access.constness = PropertyConstness::kMutable; access.const_field_info = ConstFieldInfo::None();
} }
value = jsgraph()->Constant(bit_cast<double>(value_bits)); value = jsgraph()->Constant(bit_cast<double>(value_bits));
} else { } else {
...@@ -1646,7 +1647,7 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control, ...@@ -1646,7 +1647,7 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control,
boilerplate_value.AsHeapObject().map().oddball_type() == boilerplate_value.AsHeapObject().map().oddball_type() ==
OddballType::kUninitialized; OddballType::kUninitialized;
if (is_uninitialized) { if (is_uninitialized) {
access.constness = PropertyConstness::kMutable; access.const_field_info = ConstFieldInfo::None();
} }
if (boilerplate_value.IsJSObject()) { if (boilerplate_value.IsJSObject()) {
JSObjectRef boilerplate_object = boilerplate_value.AsJSObject(); JSObjectRef boilerplate_object = boilerplate_value.AsJSObject();
......
...@@ -2231,9 +2231,6 @@ JSNativeContextSpecialization::BuildPropertyStore( ...@@ -2231,9 +2231,6 @@ JSNativeContextSpecialization::BuildPropertyStore(
AccessBuilder::ForJSObjectPropertiesOrHashKnownPointer()), AccessBuilder::ForJSObjectPropertiesOrHashKnownPointer()),
storage, effect, control); storage, effect, control);
} }
PropertyConstness constness = access_info.IsDataConstant()
? PropertyConstness::kConst
: PropertyConstness::kMutable;
bool store_to_existing_constant_field = access_info.IsDataConstant() && bool store_to_existing_constant_field = access_info.IsDataConstant() &&
access_mode == AccessMode::kStore && access_mode == AccessMode::kStore &&
!access_info.HasTransitionMap(); !access_info.HasTransitionMap();
...@@ -2246,7 +2243,7 @@ JSNativeContextSpecialization::BuildPropertyStore( ...@@ -2246,7 +2243,7 @@ JSNativeContextSpecialization::BuildPropertyStore(
MachineType::TypeForRepresentation(field_representation), MachineType::TypeForRepresentation(field_representation),
kFullWriteBarrier, kFullWriteBarrier,
LoadSensitivity::kUnsafe, LoadSensitivity::kUnsafe,
constness}; access_info.GetConstFieldInfo()};
switch (field_representation) { switch (field_representation) {
case MachineRepresentation::kFloat64: { case MachineRepresentation::kFloat64: {
...@@ -2263,7 +2260,7 @@ JSNativeContextSpecialization::BuildPropertyStore( ...@@ -2263,7 +2260,7 @@ JSNativeContextSpecialization::BuildPropertyStore(
factory()->mutable_heap_number_map()); factory()->mutable_heap_number_map());
FieldAccess value_field_access = FieldAccess value_field_access =
AccessBuilder::ForHeapNumberValue(); AccessBuilder::ForHeapNumberValue();
value_field_access.constness = field_access.constness; value_field_access.const_field_info = field_access.const_field_info;
a.Store(value_field_access, value); a.Store(value_field_access, value);
value = effect = a.Finish(); value = effect = a.Finish();
...@@ -2282,7 +2279,7 @@ JSNativeContextSpecialization::BuildPropertyStore( ...@@ -2282,7 +2279,7 @@ JSNativeContextSpecialization::BuildPropertyStore(
MachineType::TypeCompressedTaggedPointer(), MachineType::TypeCompressedTaggedPointer(),
kPointerWriteBarrier, kPointerWriteBarrier,
LoadSensitivity::kUnsafe, LoadSensitivity::kUnsafe,
constness}; access_info.GetConstFieldInfo()};
storage = effect = storage = effect =
graph()->NewNode(simplified()->LoadField(storage_access), graph()->NewNode(simplified()->LoadField(storage_access),
storage, effect, control); storage, effect, control);
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "src/compiler/common-operator.h" #include "src/compiler/common-operator.h"
#include "src/compiler/js-graph.h" #include "src/compiler/js-graph.h"
#include "src/compiler/node-properties.h" #include "src/compiler/node-properties.h"
#include "src/compiler/simplified-operator.h"
#include "src/heap/factory.h" #include "src/heap/factory.h"
#include "src/objects/objects-inl.h" #include "src/objects/objects-inl.h"
...@@ -528,11 +527,10 @@ LoadElimination::AbstractState::KillElement(Node* object, Node* index, ...@@ -528,11 +527,10 @@ LoadElimination::AbstractState::KillElement(Node* object, Node* index,
LoadElimination::AbstractState const* LoadElimination::AbstractState::AddField( LoadElimination::AbstractState const* LoadElimination::AbstractState::AddField(
Node* object, IndexRange index_range, LoadElimination::FieldInfo info, Node* object, IndexRange index_range, LoadElimination::FieldInfo info,
PropertyConstness constness, Zone* zone) const { Zone* zone) const {
AbstractState* that = new (zone) AbstractState(*this); AbstractState* that = new (zone) AbstractState(*this);
AbstractFields& fields = constness == PropertyConstness::kConst AbstractFields& fields =
? that->const_fields_ info.const_field_info.IsConst() ? that->const_fields_ : that->fields_;
: that->fields_;
for (int index : index_range) { for (int index : index_range) {
if (fields[index]) { if (fields[index]) {
fields[index] = fields[index]->Extend(object, info, zone); fields[index] = fields[index]->Extend(object, info, zone);
...@@ -603,19 +601,24 @@ LoadElimination::AbstractState const* LoadElimination::AbstractState::KillAll( ...@@ -603,19 +601,24 @@ LoadElimination::AbstractState const* LoadElimination::AbstractState::KillAll(
} }
LoadElimination::FieldInfo const* LoadElimination::AbstractState::LookupField( LoadElimination::FieldInfo const* LoadElimination::AbstractState::LookupField(
Node* object, IndexRange index_range, PropertyConstness constness) const { Node* object, IndexRange index_range,
AbstractFields const& fields = ConstFieldInfo const_field_info) const {
constness == PropertyConstness::kConst ? const_fields_ : fields_;
// Check if all the indices in {index_range} contain identical information. // Check if all the indices in {index_range} contain identical information.
// If not, a partially overlapping access has invalidated part of the value. // If not, a partially overlapping access has invalidated part of the value.
base::Optional<LoadElimination::FieldInfo const*> result; base::Optional<LoadElimination::FieldInfo const*> result;
for (int index : index_range) { for (int index : index_range) {
LoadElimination::FieldInfo const* info = nullptr; LoadElimination::FieldInfo const* info = nullptr;
if (AbstractField const* this_field = fields[index]) { if (const_field_info.IsConst()) {
info = this_field->Lookup(object); if (AbstractField const* this_field = const_fields_[index]) {
info = this_field->Lookup(object);
}
if (!(info && info->const_field_info == const_field_info)) return nullptr;
} else {
if (AbstractField const* this_field = fields_[index]) {
info = this_field->Lookup(object);
}
if (!info) return nullptr;
} }
if (!info) return nullptr;
if (!result.has_value()) { if (!result.has_value()) {
result = info; result = info;
} else { } else {
...@@ -764,10 +767,9 @@ Reduction LoadElimination::ReduceEnsureWritableFastElements(Node* node) { ...@@ -764,10 +767,9 @@ Reduction LoadElimination::ReduceEnsureWritableFastElements(Node* node) {
FieldIndexOf(JSObject::kElementsOffset, kTaggedSize), FieldIndexOf(JSObject::kElementsOffset, kTaggedSize),
MaybeHandle<Name>(), zone()); MaybeHandle<Name>(), zone());
// Add the new elements on {object}. // Add the new elements on {object}.
state = state->AddField(object, state = state->AddField(
FieldIndexOf(JSObject::kElementsOffset, kTaggedSize), object, FieldIndexOf(JSObject::kElementsOffset, kTaggedSize),
{node, MachineType::RepCompressedTaggedPointer()}, {node, MachineType::RepCompressedTaggedPointer()}, zone());
PropertyConstness::kMutable, zone());
return UpdateState(node, state); return UpdateState(node, state);
} }
...@@ -793,10 +795,9 @@ Reduction LoadElimination::ReduceMaybeGrowFastElements(Node* node) { ...@@ -793,10 +795,9 @@ Reduction LoadElimination::ReduceMaybeGrowFastElements(Node* node) {
FieldIndexOf(JSObject::kElementsOffset, kTaggedSize), FieldIndexOf(JSObject::kElementsOffset, kTaggedSize),
MaybeHandle<Name>(), zone()); MaybeHandle<Name>(), zone());
// Add the new elements on {object}. // Add the new elements on {object}.
state = state->AddField(object, state = state->AddField(
FieldIndexOf(JSObject::kElementsOffset, kTaggedSize), object, FieldIndexOf(JSObject::kElementsOffset, kTaggedSize),
{node, MachineType::RepCompressedTaggedPointer()}, {node, MachineType::RepCompressedTaggedPointer()}, zone());
PropertyConstness::kMutable, zone());
return UpdateState(node, state); return UpdateState(node, state);
} }
...@@ -885,14 +886,15 @@ Reduction LoadElimination::ReduceLoadField(Node* node, ...@@ -885,14 +886,15 @@ Reduction LoadElimination::ReduceLoadField(Node* node,
} else { } else {
IndexRange field_index = FieldIndexOf(access); IndexRange field_index = FieldIndexOf(access);
if (field_index != IndexRange::Invalid()) { if (field_index != IndexRange::Invalid()) {
PropertyConstness constness = access.constness;
MachineRepresentation representation = MachineRepresentation representation =
access.machine_type.representation(); access.machine_type.representation();
FieldInfo const* lookup_result = FieldInfo const* lookup_result =
state->LookupField(object, field_index, constness); state->LookupField(object, field_index, access.const_field_info);
if (!lookup_result && constness == PropertyConstness::kConst) { if (!lookup_result && access.const_field_info.IsConst()) {
lookup_result = state->LookupField(object, field_index, // If the access is const and we didn't find anything, also try to look
PropertyConstness::kMutable); // up information from mutable stores
lookup_result =
state->LookupField(object, field_index, ConstFieldInfo::None());
} }
if (lookup_result) { if (lookup_result) {
// Make sure we don't reuse values that were recorded with a different // Make sure we don't reuse values that were recorded with a different
...@@ -916,8 +918,9 @@ Reduction LoadElimination::ReduceLoadField(Node* node, ...@@ -916,8 +918,9 @@ Reduction LoadElimination::ReduceLoadField(Node* node,
return Replace(replacement); return Replace(replacement);
} }
} }
FieldInfo info(node, access.name, representation); FieldInfo info(node, representation, access.name,
state = state->AddField(object, field_index, info, constness, zone()); access.const_field_info);
state = state->AddField(object, field_index, info, zone());
} }
} }
Handle<Map> field_map; Handle<Map> field_map;
...@@ -950,14 +953,14 @@ Reduction LoadElimination::ReduceStoreField(Node* node, ...@@ -950,14 +953,14 @@ Reduction LoadElimination::ReduceStoreField(Node* node,
} else { } else {
IndexRange field_index = FieldIndexOf(access); IndexRange field_index = FieldIndexOf(access);
if (field_index != IndexRange::Invalid()) { if (field_index != IndexRange::Invalid()) {
PropertyConstness constness = access.constness; bool is_const_store = access.const_field_info.IsConst();
MachineRepresentation representation = MachineRepresentation representation =
access.machine_type.representation(); access.machine_type.representation();
FieldInfo const* lookup_result = FieldInfo const* lookup_result =
state->LookupField(object, field_index, constness); state->LookupField(object, field_index, access.const_field_info);
if (lookup_result && (constness == PropertyConstness::kMutable || if (lookup_result &&
V8_ENABLE_DOUBLE_CONST_STORE_CHECK_BOOL)) { (!is_const_store || V8_ENABLE_DOUBLE_CONST_STORE_CHECK_BOOL)) {
// At runtime, we should never encounter // At runtime, we should never encounter
// - any store replacing existing info with a different, incompatible // - any store replacing existing info with a different, incompatible
// representation, nor // representation, nor
...@@ -971,8 +974,7 @@ Reduction LoadElimination::ReduceStoreField(Node* node, ...@@ -971,8 +974,7 @@ Reduction LoadElimination::ReduceStoreField(Node* node,
bool incompatible_representation = bool incompatible_representation =
!lookup_result->name.is_null() && !lookup_result->name.is_null() &&
!IsCompatible(representation, lookup_result->representation); !IsCompatible(representation, lookup_result->representation);
if (incompatible_representation || if (incompatible_representation || is_const_store) {
constness == PropertyConstness::kConst) {
Node* control = NodeProperties::GetControlInput(node); Node* control = NodeProperties::GetControlInput(node);
Node* unreachable = Node* unreachable =
graph()->NewNode(common()->Unreachable(), effect, control); graph()->NewNode(common()->Unreachable(), effect, control);
...@@ -985,16 +987,16 @@ Reduction LoadElimination::ReduceStoreField(Node* node, ...@@ -985,16 +987,16 @@ Reduction LoadElimination::ReduceStoreField(Node* node,
} }
// Kill all potentially aliasing fields and record the new value. // Kill all potentially aliasing fields and record the new value.
FieldInfo new_info(new_value, access.name, representation); FieldInfo new_info(new_value, representation, access.name,
access.const_field_info);
state = state->KillField(object, field_index, access.name, zone()); state = state->KillField(object, field_index, access.name, zone());
state = state->AddField(object, field_index, new_info, state = state->AddField(object, field_index, new_info, zone());
PropertyConstness::kMutable, zone()); if (is_const_store) {
if (constness == PropertyConstness::kConst) {
// For const stores, we track information in both the const and the // For const stores, we track information in both the const and the
// mutable world to guard against field accesses that should have // mutable world to guard against field accesses that should have
// been marked const, but were not. // been marked const, but were not.
state = new_info.const_field_info = ConstFieldInfo::None();
state->AddField(object, field_index, new_info, constness, zone()); state = state->AddField(object, field_index, new_info, zone());
} }
} else { } else {
// Unsupported StoreField operator. // Unsupported StoreField operator.
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "src/codegen/machine-type.h" #include "src/codegen/machine-type.h"
#include "src/common/globals.h" #include "src/common/globals.h"
#include "src/compiler/graph-reducer.h" #include "src/compiler/graph-reducer.h"
#include "src/compiler/simplified-operator.h"
#include "src/handles/maybe-handles.h" #include "src/handles/maybe-handles.h"
#include "src/zone/zone-handle-set.h" #include "src/zone/zone-handle-set.h"
...@@ -100,21 +101,25 @@ class V8_EXPORT_PRIVATE LoadElimination final ...@@ -100,21 +101,25 @@ class V8_EXPORT_PRIVATE LoadElimination final
struct FieldInfo { struct FieldInfo {
FieldInfo() = default; FieldInfo() = default;
FieldInfo(Node* value, MachineRepresentation representation) FieldInfo(Node* value, MachineRepresentation representation,
: value(value), name(), representation(representation) {} MaybeHandle<Name> name = {},
FieldInfo(Node* value, MaybeHandle<Name> name, ConstFieldInfo const_field_info = ConstFieldInfo::None())
MachineRepresentation representation) : value(value),
: value(value), name(name), representation(representation) {} representation(representation),
name(name),
const_field_info(const_field_info) {}
bool operator==(const FieldInfo& other) const { bool operator==(const FieldInfo& other) const {
return value == other.value && name.address() == other.name.address() && return value == other.value && representation == other.representation &&
representation == other.representation; name.address() == other.name.address() &&
const_field_info == other.const_field_info;
} }
bool operator!=(const FieldInfo& other) const { return !(*this == other); } bool operator!=(const FieldInfo& other) const { return !(*this == other); }
Node* value = nullptr; Node* value = nullptr;
MaybeHandle<Name> name;
MachineRepresentation representation = MachineRepresentation::kNone; MachineRepresentation representation = MachineRepresentation::kNone;
MaybeHandle<Name> name;
ConstFieldInfo const_field_info;
}; };
// Abstract state to approximate the current state of a certain field along // Abstract state to approximate the current state of a certain field along
...@@ -235,8 +240,7 @@ class V8_EXPORT_PRIVATE LoadElimination final ...@@ -235,8 +240,7 @@ class V8_EXPORT_PRIVATE LoadElimination final
bool LookupMaps(Node* object, ZoneHandleSet<Map>* object_maps) const; bool LookupMaps(Node* object, ZoneHandleSet<Map>* object_maps) const;
AbstractState const* AddField(Node* object, IndexRange index, AbstractState const* AddField(Node* object, IndexRange index,
FieldInfo info, PropertyConstness constness, FieldInfo info, Zone* zone) const;
Zone* zone) const;
AbstractState const* KillField(const AliasStateInfo& alias_info, AbstractState const* KillField(const AliasStateInfo& alias_info,
IndexRange index, MaybeHandle<Name> name, IndexRange index, MaybeHandle<Name> name,
Zone* zone) const; Zone* zone) const;
...@@ -246,7 +250,7 @@ class V8_EXPORT_PRIVATE LoadElimination final ...@@ -246,7 +250,7 @@ class V8_EXPORT_PRIVATE LoadElimination final
Zone* zone) const; Zone* zone) const;
AbstractState const* KillAll(Zone* zone) const; AbstractState const* KillAll(Zone* zone) const;
FieldInfo const* LookupField(Node* object, IndexRange index, FieldInfo const* LookupField(Node* object, IndexRange index,
PropertyConstness constness) const; ConstFieldInfo const_field_info) const;
AbstractState const* AddElement(Node* object, Node* index, Node* value, AbstractState const* AddElement(Node* object, Node* index, Node* value,
MachineRepresentation representation, MachineRepresentation representation,
......
...@@ -203,9 +203,6 @@ Node* PropertyAccessBuilder::BuildLoadDataField( ...@@ -203,9 +203,6 @@ Node* PropertyAccessBuilder::BuildLoadDataField(
AccessBuilder::ForJSObjectPropertiesOrHashKnownPointer()), AccessBuilder::ForJSObjectPropertiesOrHashKnownPointer()),
storage, *effect, *control); storage, *effect, *control);
} }
PropertyConstness constness = access_info.IsDataConstant()
? PropertyConstness::kConst
: PropertyConstness::kMutable;
FieldAccess field_access = { FieldAccess field_access = {
kTaggedBase, kTaggedBase,
field_index.offset(), field_index.offset(),
...@@ -215,7 +212,7 @@ Node* PropertyAccessBuilder::BuildLoadDataField( ...@@ -215,7 +212,7 @@ Node* PropertyAccessBuilder::BuildLoadDataField(
MachineType::TypeForRepresentation(field_representation), MachineType::TypeForRepresentation(field_representation),
kFullWriteBarrier, kFullWriteBarrier,
LoadSensitivity::kCritical, LoadSensitivity::kCritical,
constness}; access_info.GetConstFieldInfo()};
if (field_representation == MachineRepresentation::kFloat64) { if (field_representation == MachineRepresentation::kFloat64) {
if (!field_index.is_inobject() || !FLAG_unbox_double_fields) { if (!field_index.is_inobject() || !FLAG_unbox_double_fields) {
FieldAccess const storage_access = { FieldAccess const storage_access = {
...@@ -227,7 +224,7 @@ Node* PropertyAccessBuilder::BuildLoadDataField( ...@@ -227,7 +224,7 @@ Node* PropertyAccessBuilder::BuildLoadDataField(
MachineType::TypeCompressedTaggedPointer(), MachineType::TypeCompressedTaggedPointer(),
kPointerWriteBarrier, kPointerWriteBarrier,
LoadSensitivity::kCritical, LoadSensitivity::kCritical,
constness}; access_info.GetConstFieldInfo()};
storage = *effect = graph()->NewNode( storage = *effect = graph()->NewNode(
simplified()->LoadField(storage_access), storage, *effect, *control); simplified()->LoadField(storage_access), storage, *effect, *control);
field_access.offset = HeapNumber::kValueOffset; field_access.offset = HeapNumber::kValueOffset;
......
...@@ -31,13 +31,33 @@ std::ostream& operator<<(std::ostream& os, BaseTaggedness base_taggedness) { ...@@ -31,13 +31,33 @@ std::ostream& operator<<(std::ostream& os, BaseTaggedness base_taggedness) {
UNREACHABLE(); UNREACHABLE();
} }
std::ostream& operator<<(std::ostream& os,
ConstFieldInfo const& const_field_info) {
if (const_field_info.IsConst()) {
return os << "const (field owner: " << const_field_info.owner_map.address()
<< ")";
} else {
return os << "mutable";
}
UNREACHABLE();
}
bool operator==(ConstFieldInfo const& lhs, ConstFieldInfo const& rhs) {
return lhs.owner_map.address() == rhs.owner_map.address();
}
size_t hash_value(ConstFieldInfo const& const_field_info) {
return (size_t)const_field_info.owner_map.address();
}
bool operator==(FieldAccess const& lhs, FieldAccess const& rhs) { bool operator==(FieldAccess const& lhs, FieldAccess const& rhs) {
// On purpose we don't include the write barrier kind here, as this method is // On purpose we don't include the write barrier kind here, as this method is
// really only relevant for eliminating loads and they don't care about the // really only relevant for eliminating loads and they don't care about the
// write barrier mode. // write barrier mode.
return lhs.base_is_tagged == rhs.base_is_tagged && lhs.offset == rhs.offset && return lhs.base_is_tagged == rhs.base_is_tagged && lhs.offset == rhs.offset &&
lhs.map.address() == rhs.map.address() && lhs.map.address() == rhs.map.address() &&
lhs.machine_type == rhs.machine_type; lhs.machine_type == rhs.machine_type &&
lhs.const_field_info == rhs.const_field_info;
} }
size_t hash_value(FieldAccess const& access) { size_t hash_value(FieldAccess const& access) {
...@@ -45,7 +65,7 @@ size_t hash_value(FieldAccess const& access) { ...@@ -45,7 +65,7 @@ size_t hash_value(FieldAccess const& access) {
// really only relevant for eliminating loads and they don't care about the // really only relevant for eliminating loads and they don't care about the
// write barrier mode. // write barrier mode.
return base::hash_combine(access.base_is_tagged, access.offset, return base::hash_combine(access.base_is_tagged, access.offset,
access.machine_type); access.machine_type, access.const_field_info);
} }
size_t hash_value(LoadSensitivity load_sensitivity) { size_t hash_value(LoadSensitivity load_sensitivity) {
...@@ -78,7 +98,7 @@ std::ostream& operator<<(std::ostream& os, FieldAccess const& access) { ...@@ -78,7 +98,7 @@ std::ostream& operator<<(std::ostream& os, FieldAccess const& access) {
} }
#endif #endif
os << access.type << ", " << access.machine_type << ", " os << access.type << ", " << access.machine_type << ", "
<< access.write_barrier_kind << ", " << access.constness; << access.write_barrier_kind << ", " << access.const_field_info;
if (FLAG_untrusted_code_mitigations) { if (FLAG_untrusted_code_mitigations) {
os << ", " << access.load_sensitivity; os << ", " << access.load_sensitivity;
} }
......
...@@ -44,6 +44,27 @@ size_t hash_value(LoadSensitivity); ...@@ -44,6 +44,27 @@ size_t hash_value(LoadSensitivity);
std::ostream& operator<<(std::ostream&, LoadSensitivity); std::ostream& operator<<(std::ostream&, LoadSensitivity);
struct ConstFieldInfo {
// the map that introduced the const field, if any. An access is considered
// mutable iff the handle is null.
MaybeHandle<Map> owner_map;
ConstFieldInfo() : owner_map(MaybeHandle<Map>()) {}
explicit ConstFieldInfo(Handle<Map> owner_map) : owner_map(owner_map) {}
bool IsConst() const { return !owner_map.is_null(); }
// No const field owner, i.e., a mutable field
static ConstFieldInfo None() { return ConstFieldInfo(); }
};
V8_EXPORT_PRIVATE bool operator==(ConstFieldInfo const&, ConstFieldInfo const&);
size_t hash_value(ConstFieldInfo const&);
V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream&,
ConstFieldInfo const&);
// An access descriptor for loads/stores of fixed structures like field // An access descriptor for loads/stores of fixed structures like field
// accesses of heap objects. Accesses from either tagged or untagged base // accesses of heap objects. Accesses from either tagged or untagged base
// pointers are supported; untagging is done automatically during lowering. // pointers are supported; untagging is done automatically during lowering.
...@@ -56,7 +77,8 @@ struct FieldAccess { ...@@ -56,7 +77,8 @@ struct FieldAccess {
MachineType machine_type; // machine type of the field. MachineType machine_type; // machine type of the field.
WriteBarrierKind write_barrier_kind; // write barrier hint. WriteBarrierKind write_barrier_kind; // write barrier hint.
LoadSensitivity load_sensitivity; // load safety for poisoning. LoadSensitivity load_sensitivity; // load safety for poisoning.
PropertyConstness constness; // whether the field is assigned only once ConstFieldInfo const_field_info; // the constness of this access, and the
// field owner map, if the access is const
FieldAccess() FieldAccess()
: base_is_tagged(kTaggedBase), : base_is_tagged(kTaggedBase),
...@@ -65,13 +87,13 @@ struct FieldAccess { ...@@ -65,13 +87,13 @@ struct FieldAccess {
machine_type(MachineType::None()), machine_type(MachineType::None()),
write_barrier_kind(kFullWriteBarrier), write_barrier_kind(kFullWriteBarrier),
load_sensitivity(LoadSensitivity::kUnsafe), load_sensitivity(LoadSensitivity::kUnsafe),
constness(PropertyConstness::kMutable) {} const_field_info(ConstFieldInfo::None()) {}
FieldAccess(BaseTaggedness base_is_tagged, int offset, MaybeHandle<Name> name, FieldAccess(BaseTaggedness base_is_tagged, int offset, MaybeHandle<Name> name,
MaybeHandle<Map> map, Type type, MachineType machine_type, MaybeHandle<Map> map, Type type, MachineType machine_type,
WriteBarrierKind write_barrier_kind, WriteBarrierKind write_barrier_kind,
LoadSensitivity load_sensitivity = LoadSensitivity::kUnsafe, LoadSensitivity load_sensitivity = LoadSensitivity::kUnsafe,
PropertyConstness constness = PropertyConstness::kMutable) ConstFieldInfo const_field_info = ConstFieldInfo::None())
: base_is_tagged(base_is_tagged), : base_is_tagged(base_is_tagged),
offset(offset), offset(offset),
name(name), name(name),
...@@ -80,7 +102,7 @@ struct FieldAccess { ...@@ -80,7 +102,7 @@ struct FieldAccess {
machine_type(machine_type), machine_type(machine_type),
write_barrier_kind(write_barrier_kind), write_barrier_kind(write_barrier_kind),
load_sensitivity(load_sensitivity), load_sensitivity(load_sensitivity),
constness(constness) {} const_field_info(const_field_info) {}
int tag() const { return base_is_tagged == kTaggedBase ? kHeapObjectTag : 0; } int tag() const { return base_is_tagged == kTaggedBase ? kHeapObjectTag : 0; }
}; };
......
// Copyright 2019 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
function f() {
const o = {};
// The order of the following operations is significant
o.a = 0;
o[1024] = 1; // An offset of >=1024 is required
delete o.a;
o.b = 2;
return o.b;
}
%PrepareFunctionForOptimization(f);
f();
f();
%OptimizeFunctionOnNextCall(f);
f();
function g(o) {
o.b = 2;
}
function h() {
const o = {};
o.a = 0;
o[1024] = 1;
delete o.a;
g(o);
assertEquals(o.b, 2);
}
%NeverOptimizeFunction(g);
%PrepareFunctionForOptimization(h);
h();
h();
%OptimizeFunctionOnNextCall(h);
h();
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