Skip to content

V
V8
Commit 0fb92f27 authored by bmeurer's avatar bmeurer Committed by Commit bot
Browse files
Options

[turbofan] Properly use MachineRepresentation for field access. 

When lowering DataField accesses, we should pay attention to the
MachineRepresentation reported by the AccessInfo. This doesn't
yet change the rest of the pipeline to take full advantage of
the representations.

Drive-by-fix: Make the code more robust, especially the part that
deals with mutable heap number access.

R=mvstanton@chromium.org
BUG=v8:5267,v8:5270

Review-Url: https://codereview.chromium.org/2334193002
Cr-Commit-Position: refs/heads/master@{#39381}
parent 047977c5
Hide whitespace changes
Inline Side-by-side
Showing with 92 additions and 59 deletions
src/compiler/js-native-context-specialization.cc
View file @ 0fb92f27
......@@ -881,7 +881,8 @@ JSNativeContextSpecialization::BuildPropertyAccess(
DCHECK(access_info.IsDataField());
FieldIndex const field_index = access_info.field_index();
Type* const field_type = access_info.field_type();
MachineRepresentation const rep = access_info.field_representation();
MachineRepresentation const field_representation =
access_info.field_representation();
if (access_mode == AccessMode::kLoad &&
access_info.holder().ToHandle(&holder)) {
receiver = jsgraph()->Constant(holder);
......@@ -893,18 +894,28 @@ JSNativeContextSpecialization::BuildPropertyAccess(
storage, effect, control);
}
FieldAccess field_access = {
kTaggedBase, field_index.offset(), name,
field_type, MachineType::AnyTagged(), kFullWriteBarrier};
kTaggedBase,
field_index.offset(),
name,
field_type,
MachineType::TypeForRepresentation(field_representation),
kFullWriteBarrier};
if (access_mode == AccessMode::kLoad) {
if (rep == MachineRepresentation::kFloat64) {
if (field_representation == MachineRepresentation::kFloat64) {
if (!field_index.is_inobject() || field_index.is_hidden_field() ||
!FLAG_unbox_double_fields) {
storage = effect = graph()->NewNode(
simplified()->LoadField(field_access), storage, effect, control);
FieldAccess const storage_access = {kTaggedBase,
field_index.offset(),
name,
Type::OtherInternal(),
MachineType::TaggedPointer(),
kPointerWriteBarrier};
storage = effect =
graph()->NewNode(simplified()->LoadField(storage_access), storage,
effect, control);
field_access.offset = HeapNumber::kValueOffset;
field_access.name = MaybeHandle<Name>();
}
field_access.machine_type = MachineType::Float64();
}
// TODO(turbofan): Track the field_map (if any) on the {field_access} and
// use it in LoadElimination to eliminate map checks.
......@@ -912,61 +923,83 @@ JSNativeContextSpecialization::BuildPropertyAccess(
storage, effect, control);
} else {
DCHECK_EQ(AccessMode::kStore, access_mode);
if (rep == MachineRepresentation::kFloat64) {
value = effect = graph()->NewNode(simplified()->CheckNumber(), value,
effect, control);
if (!field_index.is_inobject() || field_index.is_hidden_field() ||
!FLAG_unbox_double_fields) {
if (access_info.HasTransitionMap()) {
// Allocate a MutableHeapNumber for the new property.
effect = graph()->NewNode(
common()->BeginRegion(RegionObservability::kNotObservable),
effect);
Node* box = effect = graph()->NewNode(
simplified()->Allocate(NOT_TENURED),
jsgraph()->Constant(HeapNumber::kSize), effect, control);
effect = graph()->NewNode(
simplified()->StoreField(AccessBuilder::ForMap()), box,
jsgraph()->HeapConstant(factory()->mutable_heap_number_map()),
effect, control);
effect = graph()->NewNode(
simplified()->StoreField(AccessBuilder::ForHeapNumberValue()),
box, value, effect, control);
value = effect =
graph()->NewNode(common()->FinishRegion(), box, effect);
field_access.type = Type::TaggedPointer();
field_access.machine_type = MachineType::TaggedPointer();
} else {
// We just store directly to the MutableHeapNumber.
storage = effect =
graph()->NewNode(simplified()->LoadField(field_access), storage,
effect, control);
field_access.offset = HeapNumber::kValueOffset;
field_access.name = MaybeHandle<Name>();
field_access.machine_type = MachineType::Float64();
switch (field_representation) {
case MachineRepresentation::kFloat64: {
value = effect = graph()->NewNode(simplified()->CheckNumber(), value,
effect, control);
if (!field_index.is_inobject() || field_index.is_hidden_field() ||
!FLAG_unbox_double_fields) {
if (access_info.HasTransitionMap()) {
// Allocate a MutableHeapNumber for the new property.
effect = graph()->NewNode(
common()->BeginRegion(RegionObservability::kNotObservable),
effect);
Node* box = effect = graph()->NewNode(
simplified()->Allocate(NOT_TENURED),
jsgraph()->Constant(HeapNumber::kSize), effect, control);
effect = graph()->NewNode(
simplified()->StoreField(AccessBuilder::ForMap()), box,
jsgraph()->HeapConstant(factory()->mutable_heap_number_map()),
effect, control);
effect = graph()->NewNode(
simplified()->StoreField(AccessBuilder::ForHeapNumberValue()),
box, value, effect, control);
value = effect =
graph()->NewNode(common()->FinishRegion(), box, effect);
field_access.type = Type::TaggedPointer();
field_access.machine_type = MachineType::TaggedPointer();
field_access.write_barrier_kind = kPointerWriteBarrier;
} else {
// We just store directly to the MutableHeapNumber.
FieldAccess const storage_access = {kTaggedBase,
field_index.offset(),
name,
Type::OtherInternal(),
MachineType::TaggedPointer(),
kPointerWriteBarrier};
storage = effect =
graph()->NewNode(simplified()->LoadField(storage_access),
storage, effect, control);
field_access.offset = HeapNumber::kValueOffset;
field_access.name = MaybeHandle<Name>();
field_access.machine_type = MachineType::Float64();
}
}
} else {
// Unboxed double field, we store directly to the field.
field_access.machine_type = MachineType::Float64();
break;
}
} else if (rep == MachineRepresentation::kTaggedSigned) {
value = effect = graph()->NewNode(simplified()->CheckTaggedSigned(),
value, effect, control);
} else if (rep == MachineRepresentation::kTaggedPointer) {
// Ensure that {value} is a HeapObject.
value = effect = graph()->NewNode(simplified()->CheckTaggedPointer(),
value, effect, control);
Handle<Map> field_map;
if (access_info.field_map().ToHandle(&field_map)) {
// Emit a map check for the value.
effect = graph()->NewNode(simplified()->CheckMaps(1), value,
jsgraph()->HeapConstant(field_map), effect,
control);
case MachineRepresentation::kTaggedSigned: {
value = effect = graph()->NewNode(simplified()->CheckTaggedSigned(),
value, effect, control);
field_access.write_barrier_kind = kNoWriteBarrier;
break;
}
} else {
DCHECK(rep == MachineRepresentation::kTagged);
case MachineRepresentation::kTaggedPointer: {
// Ensure that {value} is a HeapObject.
value = effect = graph()->NewNode(simplified()->CheckTaggedPointer(),
value, effect, control);
Handle<Map> field_map;
if (access_info.field_map().ToHandle(&field_map)) {
// Emit a map check for the value.
effect = graph()->NewNode(simplified()->CheckMaps(1), value,
jsgraph()->HeapConstant(field_map),
effect, control);
}
field_access.write_barrier_kind = kPointerWriteBarrier;
break;
}
case MachineRepresentation::kTagged:
break;
case MachineRepresentation::kNone:
case MachineRepresentation::kBit:
case MachineRepresentation::kWord8:
case MachineRepresentation::kWord16:
case MachineRepresentation::kWord32:
case MachineRepresentation::kWord64:
case MachineRepresentation::kFloat32:
case MachineRepresentation::kSimd128:
UNREACHABLE();
break;
}
Handle<Map> transition_map;
if (access_info.transition_map().ToHandle(&transition_map)) {
......
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