AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert "Make LoadElimination aware of const fields (Part 2; stores)"

Browse Source 
This reverts commit e588ff10e5.

Reason for revert: consistently crashes layout tests: https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8-Blink%20Linux%2064/32225

Original change's description:
> Make LoadElimination aware of const fields (Part 2; stores)
> 
> Adds const information to store field accesses and uses it in load elimination
> 
> Change-Id: I00765c854c95c955dabd78557463267b95f75eef
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1611543
> Reviewed-by: Georg Neis <neis@chromium.org>
> Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
> Commit-Queue: Georg Schmid <gsps@google.com>
> Cr-Commit-Position: refs/heads/master@{#61796}

TBR=jarin@chromium.org,neis@chromium.org,tebbi@chromium.org,bmeurer@chromium.org,gsps@google.com

Change-Id: Ia299c36b197c2aad7cfd953b06de06f4536ddb74
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1627975
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61813}
This commit is contained in:
Clemens Hammacher 2019-05-24 06:28:07 +00:00 committed by Commit Bot
parent 2b0ac2fb9f
commit b851d753c9
11 changed files with 57 additions and 279 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/compiler/access-info.cc
View File

@ -92,11 +92,11 @@ PropertyAccessInfo PropertyAccessInfo::DataConstant(
Zone* zone, Handle<Map> receiver_map,
ZoneVector<CompilationDependencies::Dependency const*>&& dependencies,
FieldIndex field_index, Representation field_representation,
Type field_type, MaybeHandle<Map> field_map, MaybeHandle<JSObject> holder,
MaybeHandle<Map> transition_map) {
return PropertyAccessInfo(kDataConstant, holder, transition_map, field_index,
field_representation, field_type, field_map,
{{receiver_map}, zone}, std::move(dependencies));
Type field_type, MaybeHandle<Map> field_map, MaybeHandle<JSObject> holder) {
return PropertyAccessInfo(kDataConstant, holder, MaybeHandle<Map>(),
field_index, field_representation, field_type,
field_map, {{receiver_map}, zone},
std::move(dependencies));
}
// static
@ -796,22 +796,10 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
unrecorded_dependencies.push_back(
dependencies()->TransitionDependencyOffTheRecord(
MapRef(broker(), transition_map)));
// Transitioning stores *may* store to const fields. The resulting
// DataConstant access infos can be distinguished from later, i.e. redundant,
// stores to the same constant field by the presence of a transition map.
switch (details.constness()) {
case PropertyConstness::kMutable:
// Transitioning stores are never stores to constant fields.
return PropertyAccessInfo::DataField(
zone(), map, std::move(unrecorded_dependencies), field_index,
details_representation, field_type, field_map, holder,
transition_map);
case PropertyConstness::kConst:
return PropertyAccessInfo::DataConstant(
zone(), map, std::move(unrecorded_dependencies), field_index,
details_representation, field_type, field_map, holder,
transition_map);
}
UNREACHABLE();
details_representation, field_type, field_map, holder, transition_map);
}
} // namespace compiler

4
src/compiler/access-info.h
View File

@ -85,8 +85,8 @@ class PropertyAccessInfo final {
ZoneVector<CompilationDependencies::Dependency const*>&&
unrecorded_dependencies,
FieldIndex field_index, Representation field_representation,
Type field_type, MaybeHandle<Map> field_map, MaybeHandle<JSObject> holder,
MaybeHandle<Map> transition_map = MaybeHandle<Map>());
Type field_type, MaybeHandle<Map> field_map,
MaybeHandle<JSObject> holder);
static PropertyAccessInfo AccessorConstant(Zone* zone,
Handle<Map> receiver_map,
Handle<Object> constant,

36
src/compiler/js-create-lowering.cc
View File

@ -1616,39 +1616,17 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control,
DCHECK_EQ(kData, property_details.kind());
NameRef property_name = boilerplate_map.GetPropertyKey(i);
FieldIndex index = boilerplate_map.GetFieldIndexFor(i);
FieldAccess access = {kTaggedBase,
index.offset(),
property_name.object(),
MaybeHandle<Map>(),
Type::Any(),
MachineType::TypeCompressedTagged(),
kFullWriteBarrier,
LoadSensitivity::kUnsafe,
property_details.constness()};
FieldAccess access = {
kTaggedBase, index.offset(), property_name.object(),
MaybeHandle<Map>(), Type::Any(), MachineType::TypeCompressedTagged(),
kFullWriteBarrier};
Node* value;
if (boilerplate_map.IsUnboxedDoubleField(i)) {
access.machine_type = MachineType::Float64();
access.type = Type::Number();
uint64_t value_bits = boilerplate.RawFastDoublePropertyAsBitsAt(index);
if (value_bits == kHoleNanInt64) {
// This special case is analogous to is_uninitialized being true in the
// non-unboxed-double case below. The store of the hole NaN value here
// will always be followed by another store that actually initializes
// the field. The hole NaN should therefore be unobservable.
// Load elimination expects there to be at most one const store to any
// given field, so we always mark the unobservable ones as mutable.
access.constness = PropertyConstness::kMutable;
}
value = jsgraph()->Constant(bit_cast<double>(value_bits));
value = jsgraph()->Constant(boilerplate.RawFastDoublePropertyAt(index));
} else {
ObjectRef boilerplate_value = boilerplate.RawFastPropertyAt(index);
bool is_uninitialized =
boilerplate_value.IsHeapObject() &&
boilerplate_value.AsHeapObject().map().oddball_type() ==
OddballType::kUninitialized;
if (is_uninitialized) {
access.constness = PropertyConstness::kMutable;
}
if (boilerplate_value.IsJSObject()) {
JSObjectRef boilerplate_object = boilerplate_value.AsJSObject();
value = effect = AllocateFastLiteral(effect, control,
@ -1665,6 +1643,10 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control,
value = effect = builder.Finish();
} else if (property_details.representation().IsSmi()) {
// Ensure that value is stored as smi.
bool is_uninitialized =
boilerplate_value.IsHeapObject() &&
boilerplate_value.AsHeapObject().map().oddball_type() ==
OddballType::kUninitialized;
value = is_uninitialized
? jsgraph()->ZeroConstant()
: jsgraph()->Constant(boilerplate_value.AsSmi());

25
src/compiler/js-heap-broker.cc
View File

@ -208,13 +208,9 @@ void CallHandlerInfoData::Serialize(JSHeapBroker* broker) {
class JSObjectField {
public:
bool IsDouble() const { return object_ == nullptr; }
uint64_t AsBitsOfDouble() const {
CHECK(IsDouble());
return number_bits_;
}
double AsDouble() const {
CHECK(IsDouble());
return bit_cast<double>(number_bits_);
return number_;
}
bool IsObject() const { return object_ != nullptr; }
@ -223,12 +219,12 @@ class JSObjectField {
return object_;
}
explicit JSObjectField(uint64_t value_bits) : number_bits_(value_bits) {}
explicit JSObjectField(double value) : number_(value) {}
explicit JSObjectField(ObjectData* value) : object_(value) {}
private:
ObjectData* object_ = nullptr;
uint64_t number_bits_ = 0;
double number_ = 0;
};
class JSObjectData : public HeapObjectData {
@ -1797,9 +1793,8 @@ void JSObjectData::SerializeRecursive(JSHeapBroker* broker, int depth) {
DCHECK_EQ(field_index.property_index(),
static_cast<int>(inobject_fields_.size()));
if (boilerplate->IsUnboxedDoubleField(field_index)) {
uint64_t value_bits =
boilerplate->RawFastDoublePropertyAsBitsAt(field_index);
inobject_fields_.push_back(JSObjectField{value_bits});
double value = boilerplate->RawFastDoublePropertyAt(field_index);
inobject_fields_.push_back(JSObjectField{value});
} else {
Handle<Object> value(boilerplate->RawFastPropertyAt(field_index),
isolate);
@ -2366,16 +2361,6 @@ double JSObjectRef::RawFastDoublePropertyAt(FieldIndex index) const {
return object_data->GetInobjectField(index.property_index()).AsDouble();
}
uint64_t JSObjectRef::RawFastDoublePropertyAsBitsAt(FieldIndex index) const {
if (broker()->mode() == JSHeapBroker::kDisabled) {
AllowHandleDereference handle_dereference;
return object()->RawFastDoublePropertyAsBitsAt(index);
}
JSObjectData* object_data = data()->AsJSObject();
CHECK(index.is_inobject());
return object_data->GetInobjectField(index.property_index()).AsBitsOfDouble();
}
ObjectRef JSObjectRef::RawFastPropertyAt(FieldIndex index) const {
if (broker()->mode() == JSHeapBroker::kDisabled) {
AllowHandleAllocation handle_allocation;

1
src/compiler/js-heap-broker.h
View File

@ -222,7 +222,6 @@ class JSObjectRef : public HeapObjectRef {
using HeapObjectRef::HeapObjectRef;
Handle<JSObject> object() const;
uint64_t RawFastDoublePropertyAsBitsAt(FieldIndex index) const;
double RawFastDoublePropertyAt(FieldIndex index) const;
ObjectRef RawFastPropertyAt(FieldIndex index) const;

29
src/compiler/js-native-context-specialization.cc
View File

@ -2226,8 +2226,6 @@ JSNativeContextSpecialization::BuildPropertyStore(
&control, if_exceptions, access_info);
} else {
DCHECK(access_info.IsDataField() || access_info.IsDataConstant());
DCHECK(access_mode == AccessMode::kStore ||
access_mode == AccessMode::kStoreInLiteral);
FieldIndex const field_index = access_info.field_index();
Type const field_type = access_info.field_type();
MachineRepresentation const field_representation =
@ -2239,12 +2237,6 @@ JSNativeContextSpecialization::BuildPropertyStore(
simplified()->LoadField(AccessBuilder::ForJSObjectPropertiesOrHash()),
storage, effect, control);
}
PropertyConstness constness = access_info.IsDataConstant()
? PropertyConstness::kConst
: PropertyConstness::kMutable;
bool store_to_existing_constant_field = access_info.IsDataConstant() &&
access_mode == AccessMode::kStore &&
!access_info.HasTransitionMap();
FieldAccess field_access = {
kTaggedBase,
field_index.offset(),
@ -2252,10 +2244,12 @@ JSNativeContextSpecialization::BuildPropertyStore(
MaybeHandle<Map>(),
field_type,
MachineType::TypeForRepresentation(field_representation),
kFullWriteBarrier,
LoadSensitivity::kUnsafe,
constness};
kFullWriteBarrier};
bool store_to_constant_field =
(access_mode == AccessMode::kStore) && access_info.IsDataConstant();
DCHECK(access_mode == AccessMode::kStore ||
access_mode == AccessMode::kStoreInLiteral);
switch (field_representation) {
case MachineRepresentation::kFloat64: {
value = effect =
@ -2270,10 +2264,7 @@ JSNativeContextSpecialization::BuildPropertyStore(
Type::OtherInternal());
a.Store(AccessBuilder::ForMap(),
factory()->mutable_heap_number_map());
FieldAccess value_field_access =
AccessBuilder::ForHeapNumberValue();
value_field_access.constness = field_access.constness;
a.Store(value_field_access, value);
a.Store(AccessBuilder::ForHeapNumberValue(), value);
value = effect = a.Finish();
field_access.type = Type::Any();
@ -2289,9 +2280,7 @@ JSNativeContextSpecialization::BuildPropertyStore(
MaybeHandle<Map>(),
Type::OtherInternal(),
MachineType::TypeCompressedTaggedPointer(),
kPointerWriteBarrier,
LoadSensitivity::kUnsafe,
constness};
kPointerWriteBarrier};
storage = effect =
graph()->NewNode(simplified()->LoadField(storage_access),
storage, effect, control);
@ -2300,7 +2289,7 @@ JSNativeContextSpecialization::BuildPropertyStore(
field_access.machine_type = MachineType::Float64();
}
}
if (store_to_existing_constant_field) {
if (store_to_constant_field) {
DCHECK(!access_info.HasTransitionMap());
// If the field is constant check that the value we are going
// to store matches current value.
@ -2322,7 +2311,7 @@ JSNativeContextSpecialization::BuildPropertyStore(
case MachineRepresentation::kCompressedSigned:
case MachineRepresentation::kCompressedPointer:
case MachineRepresentation::kCompressed:
if (store_to_existing_constant_field) {
if (store_to_constant_field) {
DCHECK(!access_info.HasTransitionMap());
// If the field is constant check that the value we are going
// to store matches current value.

49
src/compiler/load-elimination.cc
View File

@ -601,6 +601,9 @@ Node* LoadElimination::AbstractState::LookupField(
if (AbstractField const* this_field = fields[index]) {
return this_field->Lookup(object);
}
if (constness == PropertyConstness::kConst) {
return LookupField(object, index, PropertyConstness::kMutable);
}
return nullptr;
}
@ -850,13 +853,8 @@ Reduction LoadElimination::ReduceLoadField(Node* node,
} else {
int field_index = FieldIndexOf(access);
if (field_index >= 0) {
PropertyConstness constness = access.constness;
Node* replacement = state->LookupField(object, field_index, constness);
if (!replacement && constness == PropertyConstness::kConst) {
replacement = state->LookupField(object, field_index,
PropertyConstness::kMutable);
}
if (replacement) {
if (Node* replacement =
state->LookupField(object, field_index, access.constness)) {
// Make sure we don't resurrect dead {replacement} nodes.
if (!replacement->IsDead()) {
// Introduce a TypeGuard if the type of the {replacement} node is not
@ -875,8 +873,8 @@ Reduction LoadElimination::ReduceLoadField(Node* node,
return Replace(replacement);
}
}
state = state->AddField(object, field_index, node, access.name, constness,
zone());
state = state->AddField(object, field_index, node, access.name,
access.constness, zone());
}
}
Handle<Map> field_map;
@ -909,36 +907,16 @@ Reduction LoadElimination::ReduceStoreField(Node* node,
} else {
int field_index = FieldIndexOf(access);
if (field_index >= 0) {
PropertyConstness constness = access.constness;
Node* const old_value =
state->LookupField(object, field_index, constness);
if (constness == PropertyConstness::kConst && old_value) {
// At runtime, we should never see two consecutive const stores, i.e.,
// DCHECK_NULL(old_value)
// ought to hold, but we might see such (unreachable) code statically.
Node* control = NodeProperties::GetControlInput(node);
Node* unreachable =
graph()->NewNode(common()->Unreachable(), effect, control);
return Replace(unreachable);
}
state->LookupField(object, field_index, access.constness);
if (old_value == new_value) {
// This store is fully redundant.
return Replace(effect);
}
// Kill all potentially aliasing fields and record the new value.
state = state->KillField(object, field_index, access.name, zone());
state = state->AddField(object, field_index, new_value, access.name,
PropertyConstness::kMutable, zone());
if (constness == PropertyConstness::kConst) {
// For const stores, we track information in both the const and the
// mutable world to guard against field accesses that should have
// been marked const, but were not.
state = state->AddField(object, field_index, new_value, access.name,
constness, zone());
}
access.constness, zone());
} else {
// Unsupported StoreField operator.
state = state->KillFields(object, access.name, zone());
@ -1221,13 +1199,10 @@ LoadElimination::AbstractState const* LoadElimination::ComputeLoopState(
MaybeHandle<Name>(), zone());
break;
}
case IrOpcode::kStoreField: {
FieldAccess access = FieldAccessOf(current->op());
if (access.constness == PropertyConstness::kMutable) {
state = ComputeLoopStateForStoreField(current, state, access);
}
case IrOpcode::kStoreField:
state = ComputeLoopStateForStoreField(current, state,
FieldAccessOf(current->op()));
break;
}
case IrOpcode::kStoreElement: {
Node* const object = NodeProperties::GetValueInput(current, 0);
Node* const index = NodeProperties::GetValueInput(current, 1);

2
src/compiler/simplified-operator.cc
View File

@ -78,7 +78,7 @@ std::ostream& operator<<(std::ostream& os, FieldAccess const& access) {
}
#endif
os << access.type << ", " << access.machine_type << ", "
<< access.write_barrier_kind << ", " << access.constness;
<< access.write_barrier_kind;
if (FLAG_untrusted_code_mitigations) {
os << ", " << access.load_sensitivity;
}

2
src/objects/property-details.h
View File

@ -401,8 +401,6 @@ inline PropertyConstness GeneralizeConstness(PropertyConstness a,
V8_EXPORT_PRIVATE std::ostream& operator<<(
std::ostream& os, const PropertyAttributes& attributes);
V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os,
PropertyConstness constness);
} // namespace internal
} // namespace v8

10
src/objects/property.cc
View File

@ -24,16 +24,6 @@ std::ostream& operator<<(std::ostream& os,
return os;
}
std::ostream& operator<<(std::ostream& os, PropertyConstness constness) {
switch (constness) {
case PropertyConstness::kMutable:
return os << "mutable";
case PropertyConstness::kConst:
return os << "const";
}
UNREACHABLE();
}
Descriptor::Descriptor() : details_(Smi::zero()) {}
Descriptor::Descriptor(Handle<Name> key, const MaybeObjectHandle& value,

148
test/mjsunit/compiler/load-elimination-const-field.js
View File

@ -8,149 +8,21 @@
(function() {
function maybe_sideeffect(b) { return 42; }
%NeverOptimizeFunction(maybe_sideeffect);
class B {
constructor(x) {
this.value = x;
}
}
%EnsureFeedbackVectorForFunction(B);
function lit_const_smi() {
let b = { value: 123 };
maybe_sideeffect(b);
let v1 = b.value;
maybe_sideeffect(b);
let v2 = b.value;
%TurbofanStaticAssert(Object.is(v1, v2));
%TurbofanStaticAssert(Object.is(v2, 123));
}
lit_const_smi(); lit_const_smi();
%OptimizeFunctionOnNextCall(lit_const_smi); lit_const_smi();
function lit_const_object() {
let o = {x: 123};
let b = { value: o };
maybe_sideeffect(b);
let v1 = b.value;
maybe_sideeffect(b);
let v2 = b.value;
%TurbofanStaticAssert(Object.is(v1, v2));
%TurbofanStaticAssert(Object.is(v2, o));
}
lit_const_object(); lit_const_object();
%OptimizeFunctionOnNextCall(lit_const_object); lit_const_object();
function lit_computed_smi(k) {
let kk = 2 * k;
let b = { value: kk };
maybe_sideeffect(b);
let v1 = b.value;
maybe_sideeffect(b);
let v2 = b.value;
%TurbofanStaticAssert(Object.is(v1, v2));
%TurbofanStaticAssert(Object.is(v2, kk));
}
lit_computed_smi(1); lit_computed_smi(2);
%OptimizeFunctionOnNextCall(lit_computed_smi); lit_computed_smi(3);
// TODO(bmeurer): Fix const tracking for double fields in object literals
// lit_computed_smi(1.1); lit_computed_smi(2.2);
// %OptimizeFunctionOnNextCall(lit_computed_smi); lit_computed_smi(3.3);
function lit_param_object(k) {
function f(k) {
let b = { value: k };
maybe_sideeffect(b);
let v1 = b.value;
maybe_sideeffect(b);
let v2 = b.value;
%TurbofanStaticAssert(Object.is(v1, v2));
%TurbofanStaticAssert(Object.is(v2, k));
%TurbofanStaticAssert(v1 == v2);
// TODO(gsps): Improve analysis to also propagate stored value
// Eventually, this should also work:
// %TurbofanStaticAssert(v2 == k);
}
lit_param_object({x: 1}); lit_param_object({x: 2});
%OptimizeFunctionOnNextCall(lit_param_object); lit_param_object({x: 3});
function nested_lit_param(k) {
let b = { x: { value: k } };
maybe_sideeffect(b);
let v1 = b.x.value;
maybe_sideeffect(b);
let v2 = b.x.value;
%TurbofanStaticAssert(Object.is(v1, v2));
%TurbofanStaticAssert(Object.is(v2, k));
}
nested_lit_param(1); nested_lit_param(2);
%OptimizeFunctionOnNextCall(nested_lit_param); nested_lit_param(3);
// TODO(bmeurer): Fix const tracking for double fields in object literals
// nested_lit_param(1.1); nested_lit_param(2.2);
// %OptimizeFunctionOnNextCall(nested_lit_param); nested_lit_param(3.3);
function nested_lit_param_object(k) {
let b = { x: { value: k } };
maybe_sideeffect(b);
let v1 = b.x.value;
maybe_sideeffect(b);
let v2 = b.x.value;
%TurbofanStaticAssert(Object.is(v1, v2));
%TurbofanStaticAssert(Object.is(v2, k));
}
nested_lit_param_object({x: 1}); nested_lit_param_object({x: 2});
%OptimizeFunctionOnNextCall(nested_lit_param_object);
nested_lit_param_object({x: 3});
%EnsureFeedbackVectorForFunction(inst_param);
function inst_param(k) {
let b = new B(k);
maybe_sideeffect(b);
let v1 = b.value;
maybe_sideeffect(b);
let v2 = b.value;
%TurbofanStaticAssert(Object.is(v1, v2));
%TurbofanStaticAssert(Object.is(v2, k));
}
inst_param(1); inst_param(2);
%OptimizeFunctionOnNextCall(inst_param); inst_param(3);
// TODO(gsps): Reenable once we fully support const field information
// tracking in the presence of pointer compression.
// inst_param(1.1); inst_param(2.2);
// %OptimizeFunctionOnNextCall(inst_param); inst_param(3.3);
inst_param({x: 1}); inst_param({x: 2});
%OptimizeFunctionOnNextCall(inst_param); inst_param({x: 3});
%EnsureFeedbackVectorForFunction(inst_computed);
function inst_computed(k) {
let kk = 2 * k;
let b = new B(kk);
maybe_sideeffect(b);
let v1 = b.value;
maybe_sideeffect(b);
let v2 = b.value;
%TurbofanStaticAssert(Object.is(v1, v2));
%TurbofanStaticAssert(Object.is(v2, kk));
}
inst_computed(1); inst_computed(2);
%OptimizeFunctionOnNextCall(inst_computed); inst_computed(3);
inst_computed(1.1); inst_computed(2.2);
%OptimizeFunctionOnNextCall(inst_computed); inst_computed(3.3);
%NeverOptimizeFunction(maybe_sideeffect);
f(1);
f(2);
%OptimizeFunctionOnNextCall(f);
f(3);
})();