Skip to content

Commit

Permalink
[turbofan] Use unreliable LOAD_IC feedback for Array.prototype.push.
Browse files Browse the repository at this point in the history
Add the notion of reliable vs. unreliable receiver map information to
the NodeProperties::InferReceiverMaps machinery. The information is
considered reliable here if the maps are known to be valid based on the
effect chain, and unreliable if there was a side-effect in between that
might have changed the receiver map.

Use this unreliable information for Array.prototype.push, guarded by
either stability dependencies or map checks, which might present a
potential deoptimization loop, which is very unlikely, but still needs
fixing in the future. This is important to optimize calls to push even
in cases like this

  array.push(something.func());

where we have a side-effect (the call to something.func) between the
load of array.push and the actual call.

R=jarin@chromium.org
BUG=v8:5267,v8:6241

Review-Url: https://codereview.chromium.org/2812233002
Cr-Commit-Position: refs/heads/master@{#44595}
  • Loading branch information
bmeurer authored and Commit bot committed Apr 12, 2017
1 parent a7c4e77 commit 1fceaf9
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 20 deletions.
28 changes: 25 additions & 3 deletions src/compiler/js-builtin-reducer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -912,20 +912,42 @@ Reduction JSBuiltinReducer::ReduceArrayPop(Node* node) {

// ES6 section 22.1.3.18 Array.prototype.push ( )
Reduction JSBuiltinReducer::ReduceArrayPush(Node* node) {
Handle<Map> receiver_map;
// We need exactly target, receiver and value parameters.
if (node->op()->ValueInputCount() != 3) return NoChange();
Node* receiver = NodeProperties::GetValueInput(node, 1);
Node* effect = NodeProperties::GetEffectInput(node);
Node* control = NodeProperties::GetControlInput(node);
Node* value = NodeProperties::GetValueInput(node, 2);
if (GetMapWitness(node).ToHandle(&receiver_map) &&
CanInlineArrayResizeOperation(receiver_map)) {
ZoneHandleSet<Map> receiver_maps;
NodeProperties::InferReceiverMapsResult result =
NodeProperties::InferReceiverMaps(receiver, effect, &receiver_maps);
if (receiver_maps.size() != 1) return NoChange();
DCHECK_NE(NodeProperties::kNoReceiverMaps, result);

// TODO(turbofan): Relax this to deal with multiple {receiver} maps.
Handle<Map> receiver_map = receiver_maps[0];
if (CanInlineArrayResizeOperation(receiver_map)) {
// Install code dependencies on the {receiver} prototype maps and the
// global array protector cell.
dependencies()->AssumePropertyCell(factory()->array_protector());
dependencies()->AssumePrototypeMapsStable(receiver_map);

// If the {receiver_maps} information is not reliable, we need
// to check that the {receiver} still has one of these maps.
if (result == NodeProperties::kUnreliableReceiverMaps) {
if (receiver_map->is_stable()) {
dependencies()->AssumeMapStable(receiver_map);
} else {
// TODO(turbofan): This is a potential - yet unlikely - deoptimization
// loop, since we might not learn from this deoptimization in baseline
// code. We need a way to learn from deoptimizations in optimized to
// address these problems.
effect = graph()->NewNode(
simplified()->CheckMaps(CheckMapsFlag::kNone, receiver_maps),
receiver, effect, control);
}
}

// TODO(turbofan): Perform type checks on the {value}. We are not guaranteed
// to learn from these checks in case they fail, as the witness (i.e. the
// map check from the LoadIC for a.push) might not be executed in baseline
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/js-call-reducer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,9 @@ Reduction JSCallReducer::ReduceObjectPrototypeGetProto(Node* node) {

// Try to determine the {receiver} map.
ZoneHandleSet<Map> receiver_maps;
if (NodeProperties::InferReceiverMaps(receiver, effect, &receiver_maps)) {
NodeProperties::InferReceiverMapsResult result =
NodeProperties::InferReceiverMaps(receiver, effect, &receiver_maps);
if (result == NodeProperties::kReliableReceiverMaps) {
Handle<Map> candidate_map(
receiver_maps[0]->GetPrototypeChainRootMap(isolate()));
Handle<Object> candidate_prototype(candidate_map->prototype(), isolate());
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/js-inlining.cc
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,12 @@ bool NeedsConvertReceiver(Node* receiver, Node* effect) {
return false;
}
default: {
// We don't really care about the exact maps here, just the instance
// types, which don't change across potential side-effecting operations.
ZoneHandleSet<Map> maps;
if (NodeProperties::InferReceiverMaps(receiver, effect, &maps)) {
NodeProperties::InferReceiverMapsResult result =
NodeProperties::InferReceiverMaps(receiver, effect, &maps);
if (result != NodeProperties::kNoReceiverMaps) {
// Check if all {maps} are actually JSReceiver maps.
for (size_t i = 0; i < maps.size(); ++i) {
if (!maps[i]->IsJSReceiverMap()) return true;
Expand Down
14 changes: 13 additions & 1 deletion src/compiler/js-native-context-specialization.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2267,7 +2267,19 @@ bool JSNativeContextSpecialization::ExtractReceiverMaps(
bool JSNativeContextSpecialization::InferReceiverMaps(
Node* receiver, Node* effect, MapHandleList* receiver_maps) {
ZoneHandleSet<Map> maps;
if (NodeProperties::InferReceiverMaps(receiver, effect, &maps)) {
NodeProperties::InferReceiverMapsResult result =
NodeProperties::InferReceiverMaps(receiver, effect, &maps);
if (result == NodeProperties::kReliableReceiverMaps) {
for (size_t i = 0; i < maps.size(); ++i) {
receiver_maps->Add(maps[i]);
}
return true;
} else if (result == NodeProperties::kUnreliableReceiverMaps) {
// For untrusted receiver maps, we can still use the information
// if the maps are stable.
for (size_t i = 0; i < maps.size(); ++i) {
if (!maps[i]->is_stable()) return false;
}
for (size_t i = 0; i < maps.size(); ++i) {
receiver_maps->Add(maps[i]);
}
Expand Down
29 changes: 18 additions & 11 deletions src/compiler/node-properties.cc
Original file line number Diff line number Diff line change
Expand Up @@ -333,23 +333,26 @@ bool NodeProperties::IsSame(Node* a, Node* b) {
}

// static
bool NodeProperties::InferReceiverMaps(Node* receiver, Node* effect,
ZoneHandleSet<Map>* maps_return) {
NodeProperties::InferReceiverMapsResult NodeProperties::InferReceiverMaps(
Node* receiver, Node* effect, ZoneHandleSet<Map>* maps_return) {
HeapObjectMatcher m(receiver);
if (m.HasValue()) {
Handle<Map> receiver_map(m.Value()->map());
if (receiver_map->is_stable()) {
// The {receiver_map} is only reliable when we install a stability
// code dependency.
*maps_return = ZoneHandleSet<Map>(receiver_map);
return true;
return kUnreliableReceiverMaps;
}
}
InferReceiverMapsResult result = kReliableReceiverMaps;
while (true) {
switch (effect->opcode()) {
case IrOpcode::kCheckMaps: {
Node* const object = GetValueInput(effect, 0);
if (IsSame(receiver, object)) {
*maps_return = CheckMapsParametersOf(effect->op()).maps();
return true;
return result;
}
break;
}
Expand All @@ -365,12 +368,12 @@ bool NodeProperties::InferReceiverMaps(Node* receiver, Node* effect,
if (initial_map->constructor_or_backpointer() ==
*mtarget.Value()) {
*maps_return = ZoneHandleSet<Map>(initial_map);
return true;
return result;
}
}
}
// We reached the allocation of the {receiver}.
return false;
return kNoReceiverMaps;
}
break;
}
Expand All @@ -385,12 +388,12 @@ bool NodeProperties::InferReceiverMaps(Node* receiver, Node* effect,
HeapObjectMatcher m(value);
if (m.HasValue()) {
*maps_return = ZoneHandleSet<Map>(Handle<Map>::cast(m.Value()));
return true;
return result;
}
}
// Without alias analysis we cannot tell whether this
// StoreField[map] affects {receiver} or not.
return false;
result = kUnreliableReceiverMaps;
}
break;
}
Expand All @@ -403,10 +406,14 @@ bool NodeProperties::InferReceiverMaps(Node* receiver, Node* effect,
}
default: {
DCHECK_EQ(1, effect->op()->EffectOutputCount());
if (effect->op()->EffectInputCount() != 1 ||
!effect->op()->HasProperty(Operator::kNoWrite)) {
if (effect->op()->EffectInputCount() != 1) {
// Didn't find any appropriate CheckMaps node.
return false;
return kNoReceiverMaps;
}
if (!effect->op()->HasProperty(Operator::kNoWrite)) {
// Without alias/escape analysis we cannot tell whether this
// {effect} affects {receiver} or not.
result = kUnreliableReceiverMaps;
}
break;
}
Expand Down
11 changes: 8 additions & 3 deletions src/compiler/node-properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,15 @@ class V8_EXPORT_PRIVATE NodeProperties final {
static bool IsSame(Node* a, Node* b);

// Walks up the {effect} chain to find a witness that provides map
// information about the {receiver}. Doesn't look through potentially
// information about the {receiver}. Can look through potentially
// side effecting nodes.
static bool InferReceiverMaps(Node* receiver, Node* effect,
ZoneHandleSet<Map>* maps_return);
enum InferReceiverMapsResult {
kNoReceiverMaps, // No receiver maps inferred.
kReliableReceiverMaps, // Receiver maps can be trusted.
kUnreliableReceiverMaps // Receiver maps might have changed (side-effect).
};
static InferReceiverMapsResult InferReceiverMaps(
Node* receiver, Node* effect, ZoneHandleSet<Map>* maps_return);

// ---------------------------------------------------------------------------
// Context.
Expand Down

0 comments on commit 1fceaf9

Please sign in to comment.