Commit 58164026 authored by Mythri A's avatar Mythri A Committed by Commit Bot

[turboprop] Fix type info to also expect Smi for double data-field loads

An earlier cl:
https://chromium-review.googlesource.com/c/v8/v8/+/2354810 fixed loading
double fields with dynamic map checks. The fix however didn't update the
expected type information to also expect Smi fields. So, in the later
optimization phases the CheckHeapObject operation was reduced to a NoP
since the expected type was a HeapObject. This cl fixes the bug by
updating the type to Type::Any.

Bug: chromium:1124696, v8:10582
Change-Id: Ic96dd74c97caf8eaf5289d8e8939f6feb1686a57
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2396088
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69730}
parent 0c7516e0
......@@ -199,18 +199,19 @@ Node* PropertyAccessBuilder::BuildLoadDataField(NameRef const& name,
MachineRepresentation::kFloat64) {
bool const is_heapnumber = !is_inobject || !FLAG_unbox_double_fields;
if (is_heapnumber) {
FieldAccess const storage_access = {kTaggedBase,
field_access.offset,
name.object(),
MaybeHandle<Map>(),
Type::OtherInternal(),
MachineType::TaggedPointer(),
kPointerWriteBarrier,
LoadSensitivity::kCritical,
field_access.const_field_info};
storage = *effect = graph()->NewNode(
simplified()->LoadField(storage_access), storage, *effect, *control);
if (dependencies() == nullptr) {
FieldAccess const storage_access = {kTaggedBase,
field_access.offset,
name.object(),
MaybeHandle<Map>(),
Type::Any(),
MachineType::AnyTagged(),
kPointerWriteBarrier,
LoadSensitivity::kCritical,
field_access.const_field_info};
storage = *effect =
graph()->NewNode(simplified()->LoadField(storage_access), storage,
*effect, *control);
// We expect the loaded value to be a heap number here. With
// in-place field representation changes it is possible this is a
// no longer a heap number without map transitions. If we haven't taken
......@@ -227,6 +228,19 @@ Node* PropertyAccessBuilder::BuildLoadDataField(NameRef const& name,
*effect = graph()->NewNode(
simplified()->CheckIf(DeoptimizeReason::kNotAHeapNumber),
is_heap_number, *effect, *control);
} else {
FieldAccess const storage_access = {kTaggedBase,
field_access.offset,
name.object(),
MaybeHandle<Map>(),
Type::OtherInternal(),
MachineType::TaggedPointer(),
kPointerWriteBarrier,
LoadSensitivity::kCritical,
field_access.const_field_info};
storage = *effect =
graph()->NewNode(simplified()->LoadField(storage_access), storage,
*effect, *control);
}
field_access.offset = HeapNumber::kValueOffset;
field_access.name = MaybeHandle<Name>();
......
......@@ -17,3 +17,16 @@ v.b = {x: 20};
assertEquals(f(v).x, 20);
// Must deoptimize because of field-rep changes for field 'b'
assertUnoptimized(f);
function f0(v) {
return v.b;
}
var v0 = { b: 10.23 };
%PrepareFunctionForOptimization(f0);
f0(v0);
// Transition the field to an Smi field.
v0.b = {};
v0.b = 0;
%OptimizeFunctionOnNextCall(f0);
f0(v0);
assertEquals(f0(v0), 0);
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