Commit c7ca8dac authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[turbofan] Use CheckHeapObject only for StoreField.

Previously we had to use CheckHeapObject in front of every CheckMaps,
CompareMaps and TransitionElementsKind operation. Now these operators
request HeapObject representation themselves (requiring for CompareMaps
and TransitionElementsKind to remove the kNoDeopt property). This means
we only do CheckHeapObject for StoreField to a field that has HeapObject
representation.

This not only leads to smaller graphs in the compiler, but also removes
most uses of the CheckHeapObject operator, which doesn't express a real
semantic property in the compiler frontend.

Bug: v8:9183, v8:9250
Refs: nodejs/node#27667
Change-Id: Ie3d83de69583b1bed6c1c53444bfc97aaef624bb
Cq-Include-Trybots: luci.chromium.try:linux-rel,win7-rel
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1612902Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61508}
parent 099669ec
...@@ -432,8 +432,6 @@ Reduction JSNativeContextSpecialization::ReduceJSInstanceOf(Node* node) { ...@@ -432,8 +432,6 @@ Reduction JSNativeContextSpecialization::ReduceJSInstanceOf(Node* node) {
kStartAtPrototype); kStartAtPrototype);
// Monomorphic property access. // Monomorphic property access.
constructor =
access_builder.BuildCheckHeapObject(constructor, &effect, control);
access_builder.BuildCheckMaps(constructor, &effect, control, access_builder.BuildCheckMaps(constructor, &effect, control,
access_info.receiver_maps()); access_info.receiver_maps());
...@@ -1124,8 +1122,6 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess( ...@@ -1124,8 +1122,6 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
effect = effect =
graph()->NewNode(common()->EffectPhi(2), etrue, efalse, control); graph()->NewNode(common()->EffectPhi(2), etrue, efalse, control);
} else { } else {
receiver =
access_builder.BuildCheckHeapObject(receiver, &effect, control);
access_builder.BuildCheckMaps(receiver, &effect, control, access_builder.BuildCheckMaps(receiver, &effect, control,
access_info.receiver_maps()); access_info.receiver_maps());
} }
...@@ -1154,7 +1150,7 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess( ...@@ -1154,7 +1150,7 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
} }
} }
// Ensure that {receiver} is a heap object. // Handle the case that {receiver} may be a number.
Node* receiverissmi_control = nullptr; Node* receiverissmi_control = nullptr;
Node* receiverissmi_effect = effect; Node* receiverissmi_effect = effect;
if (receiverissmi_possible) { if (receiverissmi_possible) {
...@@ -1163,9 +1159,6 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess( ...@@ -1163,9 +1159,6 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
control = graph()->NewNode(common()->IfFalse(), branch); control = graph()->NewNode(common()->IfFalse(), branch);
receiverissmi_control = graph()->NewNode(common()->IfTrue(), branch); receiverissmi_control = graph()->NewNode(common()->IfTrue(), branch);
receiverissmi_effect = effect; receiverissmi_effect = effect;
} else {
receiver =
access_builder.BuildCheckHeapObject(receiver, &effect, control);
} }
// Generate code for the various different property access patterns. // Generate code for the various different property access patterns.
...@@ -1547,10 +1540,6 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess( ...@@ -1547,10 +1540,6 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
} }
} }
// Ensure that {receiver} is a heap object.
PropertyAccessBuilder access_builder(jsgraph(), broker(), dependencies());
receiver = access_builder.BuildCheckHeapObject(receiver, &effect, control);
// Check if we have the necessary data for building element accesses. // Check if we have the necessary data for building element accesses.
for (ElementAccessInfo const& access_info : access_infos) { for (ElementAccessInfo const& access_info : access_infos) {
if (!IsFixedTypedArrayElementsKind(access_info.elements_kind())) continue; if (!IsFixedTypedArrayElementsKind(access_info.elements_kind())) continue;
...@@ -1567,6 +1556,7 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess( ...@@ -1567,6 +1556,7 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
} }
// Check for the monomorphic case. // Check for the monomorphic case.
PropertyAccessBuilder access_builder(jsgraph(), broker(), dependencies());
if (access_infos.size() == 1) { if (access_infos.size() == 1) {
ElementAccessInfo access_info = access_infos.front(); ElementAccessInfo access_info = access_infos.front();
...@@ -1915,8 +1905,6 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess( ...@@ -1915,8 +1905,6 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
effect = effect =
graph()->NewNode(common()->EffectPhi(2), etrue, efalse, control); graph()->NewNode(common()->EffectPhi(2), etrue, efalse, control);
} else { } else {
receiver =
access_builder.BuildCheckHeapObject(receiver, &effect, control);
access_builder.BuildCheckMaps(receiver, &effect, control, access_builder.BuildCheckMaps(receiver, &effect, control,
access_info.receiver_maps()); access_info.receiver_maps());
} }
...@@ -1945,7 +1933,7 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess( ...@@ -1945,7 +1933,7 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
} }
} }
// Ensure that {receiver} is a heap object. // Handle the case that {receiver} may be a number.
Node* receiverissmi_control = nullptr; Node* receiverissmi_control = nullptr;
Node* receiverissmi_effect = effect; Node* receiverissmi_effect = effect;
if (receiverissmi_possible) { if (receiverissmi_possible) {
...@@ -1954,9 +1942,6 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess( ...@@ -1954,9 +1942,6 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
control = graph()->NewNode(common()->IfFalse(), branch); control = graph()->NewNode(common()->IfFalse(), branch);
receiverissmi_control = graph()->NewNode(common()->IfTrue(), branch); receiverissmi_control = graph()->NewNode(common()->IfTrue(), branch);
receiverissmi_effect = effect; receiverissmi_effect = effect;
} else {
receiver =
access_builder.BuildCheckHeapObject(receiver, &effect, control);
} }
// Generate code for the various different property access patterns. // Generate code for the various different property access patterns.
...@@ -2602,8 +2587,6 @@ JSNativeContextSpecialization::BuildPropertyStore( ...@@ -2602,8 +2587,6 @@ JSNativeContextSpecialization::BuildPropertyStore(
MachineRepresentation::kTaggedPointer || MachineRepresentation::kTaggedPointer ||
field_representation == field_representation ==
MachineRepresentation::kCompressedPointer) { MachineRepresentation::kCompressedPointer) {
// Ensure that {value} is a HeapObject.
value = access_builder.BuildCheckHeapObject(value, &effect, control);
Handle<Map> field_map; Handle<Map> field_map;
if (access_info.field_map().ToHandle(&field_map)) { if (access_info.field_map().ToHandle(&field_map)) {
// Emit a map check for the value. // Emit a map check for the value.
...@@ -2611,6 +2594,10 @@ JSNativeContextSpecialization::BuildPropertyStore( ...@@ -2611,6 +2594,10 @@ JSNativeContextSpecialization::BuildPropertyStore(
simplified()->CheckMaps(CheckMapsFlag::kNone, simplified()->CheckMaps(CheckMapsFlag::kNone,
ZoneHandleSet<Map>(field_map)), ZoneHandleSet<Map>(field_map)),
value, effect, control); value, effect, control);
} else {
// Ensure that {value} is a HeapObject.
value = effect = graph()->NewNode(simplified()->CheckHeapObject(),
value, effect, control);
} }
field_access.write_barrier_kind = kPointerWriteBarrier; field_access.write_barrier_kind = kPointerWriteBarrier;
...@@ -2721,7 +2708,6 @@ Reduction JSNativeContextSpecialization::ReduceJSStoreDataPropertyInLiteral( ...@@ -2721,7 +2708,6 @@ Reduction JSNativeContextSpecialization::ReduceJSStoreDataPropertyInLiteral(
// Monomorphic property access. // Monomorphic property access.
PropertyAccessBuilder access_builder(jsgraph(), broker(), dependencies()); PropertyAccessBuilder access_builder(jsgraph(), broker(), dependencies());
receiver = access_builder.BuildCheckHeapObject(receiver, &effect, control);
access_builder.BuildCheckMaps(receiver, &effect, control, access_builder.BuildCheckMaps(receiver, &effect, control,
access_info.receiver_maps()); access_info.receiver_maps());
......
...@@ -81,60 +81,6 @@ bool PropertyAccessBuilder::TryBuildNumberCheck( ...@@ -81,60 +81,6 @@ bool PropertyAccessBuilder::TryBuildNumberCheck(
return false; return false;
} }
namespace {
bool NeedsCheckHeapObject(Node* receiver) {
switch (receiver->opcode()) {
case IrOpcode::kConvertReceiver:
case IrOpcode::kHeapConstant:
case IrOpcode::kJSCloneObject:
case IrOpcode::kJSConstruct:
case IrOpcode::kJSConstructForwardVarargs:
case IrOpcode::kJSConstructWithArrayLike:
case IrOpcode::kJSConstructWithSpread:
case IrOpcode::kJSCreate:
case IrOpcode::kJSCreateArguments:
case IrOpcode::kJSCreateArray:
case IrOpcode::kJSCreateArrayFromIterable:
case IrOpcode::kJSCreateArrayIterator:
case IrOpcode::kJSCreateAsyncFunctionObject:
case IrOpcode::kJSCreateBoundFunction:
case IrOpcode::kJSCreateClosure:
case IrOpcode::kJSCreateCollectionIterator:
case IrOpcode::kJSCreateEmptyLiteralArray:
case IrOpcode::kJSCreateEmptyLiteralObject:
case IrOpcode::kJSCreateGeneratorObject:
case IrOpcode::kJSCreateIterResultObject:
case IrOpcode::kJSCreateKeyValueArray:
case IrOpcode::kJSCreateLiteralArray:
case IrOpcode::kJSCreateLiteralObject:
case IrOpcode::kJSCreateLiteralRegExp:
case IrOpcode::kJSCreateObject:
case IrOpcode::kJSCreatePromise:
case IrOpcode::kJSCreateStringIterator:
case IrOpcode::kJSCreateTypedArray:
case IrOpcode::kJSGetSuperConstructor:
case IrOpcode::kJSToName:
case IrOpcode::kJSToObject:
case IrOpcode::kJSToString:
case IrOpcode::kTypeOf:
return false;
default:
return true;
}
}
} // namespace
Node* PropertyAccessBuilder::BuildCheckHeapObject(Node* receiver, Node** effect,
Node* control) {
if (NeedsCheckHeapObject(receiver)) {
receiver = *effect = graph()->NewNode(simplified()->CheckHeapObject(),
receiver, *effect, control);
}
return receiver;
}
void PropertyAccessBuilder::BuildCheckMaps( void PropertyAccessBuilder::BuildCheckMaps(
Node* receiver, Node** effect, Node* control, Node* receiver, Node** effect, Node* control,
ZoneVector<Handle<Map>> const& receiver_maps) { ZoneVector<Handle<Map>> const& receiver_maps) {
......
...@@ -41,7 +41,6 @@ class PropertyAccessBuilder { ...@@ -41,7 +41,6 @@ class PropertyAccessBuilder {
ZoneVector<Handle<Map>> const& maps, Node** receiver, ZoneVector<Handle<Map>> const& maps, Node** receiver,
Node** effect, Node* control); Node** effect, Node* control);
Node* BuildCheckHeapObject(Node* receiver, Node** effect, Node* control);
void BuildCheckMaps(Node* receiver, Node** effect, Node* control, void BuildCheckMaps(Node* receiver, Node** effect, Node* control,
ZoneVector<Handle<Map>> const& receiver_maps); ZoneVector<Handle<Map>> const& receiver_maps);
Node* BuildCheckValue(Node* receiver, Node** effect, Node* control, Node* BuildCheckValue(Node* receiver, Node** effect, Node* control,
......
...@@ -3281,14 +3281,21 @@ class RepresentationSelector { ...@@ -3281,14 +3281,21 @@ class RepresentationSelector {
case IrOpcode::kMapGuard: case IrOpcode::kMapGuard:
// Eliminate MapGuard nodes here. // Eliminate MapGuard nodes here.
return VisitUnused(node); return VisitUnused(node);
case IrOpcode::kCheckMaps: case IrOpcode::kCheckMaps: {
CheckMapsParameters const& p = CheckMapsParametersOf(node->op());
return VisitUnop(
node, UseInfo::CheckedHeapObjectAsTaggedPointer(p.feedback()),
MachineRepresentation::kNone);
}
case IrOpcode::kTransitionElementsKind: { case IrOpcode::kTransitionElementsKind: {
VisitInputs(node); return VisitUnop(
return SetOutput(node, MachineRepresentation::kNone); node, UseInfo::CheckedHeapObjectAsTaggedPointer(VectorSlotPair()),
MachineRepresentation::kNone);
} }
case IrOpcode::kCompareMaps: case IrOpcode::kCompareMaps:
return VisitUnop(node, UseInfo::AnyTagged(), return VisitUnop(
MachineRepresentation::kBit); node, UseInfo::CheckedHeapObjectAsTaggedPointer(VectorSlotPair()),
MachineRepresentation::kBit);
case IrOpcode::kEnsureWritableFastElements: case IrOpcode::kEnsureWritableFastElements:
return VisitBinop(node, UseInfo::AnyTagged(), return VisitBinop(node, UseInfo::AnyTagged(),
MachineRepresentation::kTaggedPointer); MachineRepresentation::kTaggedPointer);
......
...@@ -1377,7 +1377,7 @@ const Operator* SimplifiedOperatorBuilder::CompareMaps( ...@@ -1377,7 +1377,7 @@ const Operator* SimplifiedOperatorBuilder::CompareMaps(
DCHECK_LT(0, maps.size()); DCHECK_LT(0, maps.size());
return new (zone()) Operator1<ZoneHandleSet<Map>>( // -- return new (zone()) Operator1<ZoneHandleSet<Map>>( // --
IrOpcode::kCompareMaps, // opcode IrOpcode::kCompareMaps, // opcode
Operator::kEliminatable, // flags Operator::kNoThrow | Operator::kNoWrite, // flags
"CompareMaps", // name "CompareMaps", // name
1, 1, 1, 1, 1, 0, // counts 1, 1, 1, 1, 1, 0, // counts
maps); // parameter maps); // parameter
...@@ -1462,7 +1462,7 @@ const Operator* SimplifiedOperatorBuilder::TransitionElementsKind( ...@@ -1462,7 +1462,7 @@ const Operator* SimplifiedOperatorBuilder::TransitionElementsKind(
ElementsTransition transition) { ElementsTransition transition) {
return new (zone()) Operator1<ElementsTransition>( // -- return new (zone()) Operator1<ElementsTransition>( // --
IrOpcode::kTransitionElementsKind, // opcode IrOpcode::kTransitionElementsKind, // opcode
Operator::kNoDeopt | Operator::kNoThrow, // flags Operator::kNoThrow, // flags
"TransitionElementsKind", // name "TransitionElementsKind", // name
1, 1, 1, 0, 1, 0, // counts 1, 1, 1, 0, 1, 0, // counts
transition); // parameter transition); // parameter
......
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