Skip to content

Commit

Permalink
[turbofan] Load elimination: do not kill may-alias node maps on map c…
Browse files Browse the repository at this point in the history 
…hecks.

Instead we only change the map for the node being checked.

This also changes AbstractMaps to look through renames for keys. That
might theoretically lead to seeing less precise types for MayAlias
tests, the hope is it does not matter much.

Bug: v8:6396
Change-Id: I28a067080a3bc58c62a9dd5a76dce1aa348d8e0c
Reviewed-on: https://chromium-review.googlesource.com/730705
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48785}
  • Loading branch information
@jaro-sevcik
jaro-sevcik authored and Commit Bot committed Oct 20, 2017
1 parent d5c19aa commit 9884bc5
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 43 deletions.
94 changes: 57 additions & 37 deletions src/compiler/load-elimination.cc
View file
Expand Up @@ -17,28 +17,43 @@ namespace compiler {


namespace { namespace {


enum Aliasing { kNoAlias, kMayAlias, kMustAlias }; bool IsRename(Node* node) {
switch (node->opcode()) {
case IrOpcode::kFinishRegion:
case IrOpcode::kTypeGuard:
return true;
default:
return false;
}
}

Node* ResolveRenames(Node* node) {
while (IsRename(node)) {
node = node->InputAt(0);
}
return node;
}


Aliasing QueryAlias(Node* a, Node* b) { bool MayAlias(Node* a, Node* b) {
if (a == b) return kMustAlias; if (a == b) return true;
if (!NodeProperties::GetType(a)->Maybe(NodeProperties::GetType(b))) { if (!NodeProperties::GetType(a)->Maybe(NodeProperties::GetType(b))) {
return kNoAlias; return false;
} }
switch (b->opcode()) { switch (b->opcode()) {
case IrOpcode::kAllocate: { case IrOpcode::kAllocate: {
switch (a->opcode()) { switch (a->opcode()) {
case IrOpcode::kAllocate: case IrOpcode::kAllocate:
case IrOpcode::kHeapConstant: case IrOpcode::kHeapConstant:
case IrOpcode::kParameter: case IrOpcode::kParameter:
return kNoAlias; return false;
default: default:
break; break;
} }
break; break;
} }
case IrOpcode::kFinishRegion: case IrOpcode::kFinishRegion:
case IrOpcode::kTypeGuard: case IrOpcode::kTypeGuard:
return QueryAlias(a, b->InputAt(0)); return MayAlias(a, b->InputAt(0));
default: default:
break; break;
} }
Expand All @@ -47,24 +62,24 @@ Aliasing QueryAlias(Node* a, Node* b) {
switch (b->opcode()) { switch (b->opcode()) {
case IrOpcode::kHeapConstant: case IrOpcode::kHeapConstant:
case IrOpcode::kParameter: case IrOpcode::kParameter:
return kNoAlias; return false;
default: default:
break; break;
} }
break; break;
} }
case IrOpcode::kFinishRegion: case IrOpcode::kFinishRegion:
case IrOpcode::kTypeGuard: case IrOpcode::kTypeGuard:
return QueryAlias(a->InputAt(0), b); return MayAlias(a->InputAt(0), b);
default: default:
break; break;
} }
return kMayAlias; return true;
} }


bool MayAlias(Node* a, Node* b) { return QueryAlias(a, b) != kNoAlias; } bool MustAlias(Node* a, Node* b) {

return ResolveRenames(a) == ResolveRenames(b);
bool MustAlias(Node* a, Node* b) { return QueryAlias(a, b) == kMustAlias; } }


} // namespace } // namespace


Expand Down Expand Up @@ -371,15 +386,22 @@ void LoadElimination::AbstractField::Print() const {
} }
} }


LoadElimination::AbstractMaps::AbstractMaps(Zone* zone)
: info_for_node_(zone) {}

LoadElimination::AbstractMaps::AbstractMaps(Node* object,
ZoneHandleSet<Map> maps, Zone* zone)
: info_for_node_(zone) {
object = ResolveRenames(object);
info_for_node_.insert(std::make_pair(object, maps));
}

bool LoadElimination::AbstractMaps::Lookup( bool LoadElimination::AbstractMaps::Lookup(
Node* object, ZoneHandleSet<Map>* object_maps) const { Node* object, ZoneHandleSet<Map>* object_maps) const {
for (auto pair : info_for_node_) { auto it = info_for_node_.find(ResolveRenames(object));
if (MustAlias(object, pair.first)) { if (it == info_for_node_.end()) return false;
*object_maps = pair.second; *object_maps = it->second;
return true; return true;
}
}
return false;
} }


LoadElimination::AbstractMaps const* LoadElimination::AbstractMaps::Kill( LoadElimination::AbstractMaps const* LoadElimination::AbstractMaps::Kill(
Expand Down Expand Up @@ -415,7 +437,8 @@ LoadElimination::AbstractMaps const* LoadElimination::AbstractMaps::Extend(
Node* object, ZoneHandleSet<Map> maps, Zone* zone) const { Node* object, ZoneHandleSet<Map> maps, Zone* zone) const {
AbstractMaps* that = new (zone) AbstractMaps(zone); AbstractMaps* that = new (zone) AbstractMaps(zone);
that->info_for_node_ = this->info_for_node_; that->info_for_node_ = this->info_for_node_;
that->info_for_node_.insert(std::make_pair(object, maps)); object = ResolveRenames(object);
that->info_for_node_[object] = maps;
return that; return that;
} }


Expand Down Expand Up @@ -516,7 +539,7 @@ bool LoadElimination::AbstractState::LookupMaps(
return this->maps_ && this->maps_->Lookup(object, object_map); return this->maps_ && this->maps_->Lookup(object, object_map);
} }


LoadElimination::AbstractState const* LoadElimination::AbstractState::AddMaps( LoadElimination::AbstractState const* LoadElimination::AbstractState::SetMaps(
Node* object, ZoneHandleSet<Map> maps, Zone* zone) const { Node* object, ZoneHandleSet<Map> maps, Zone* zone) const {
AbstractState* that = new (zone) AbstractState(*this); AbstractState* that = new (zone) AbstractState(*this);
if (that->maps_) { if (that->maps_) {
Expand Down Expand Up @@ -650,9 +673,11 @@ Node* LoadElimination::AbstractState::LookupField(Node* object,
} }


bool LoadElimination::AliasStateInfo::MayAlias(Node* other) const { bool LoadElimination::AliasStateInfo::MayAlias(Node* other) const {
if (QueryAlias(object_, other) == kNoAlias) { // Decide aliasing based on the node kinds.
if (!compiler::MayAlias(object_, other)) {
return false; return false;
} }
// Decide aliasing based on maps (if available).
Handle<Map> map; Handle<Map> map;
if (map_.ToHandle(&map)) { if (map_.ToHandle(&map)) {
ZoneHandleSet<Map> other_maps; ZoneHandleSet<Map> other_maps;
Expand Down Expand Up @@ -721,10 +746,9 @@ Reduction LoadElimination::ReduceMapGuard(Node* node) {
ZoneHandleSet<Map> object_maps; ZoneHandleSet<Map> object_maps;
if (state->LookupMaps(object, &object_maps)) { if (state->LookupMaps(object, &object_maps)) {
if (maps.contains(object_maps)) return Replace(effect); if (maps.contains(object_maps)) return Replace(effect);
state = state->KillMaps(object, zone());
// TODO(turbofan): Compute the intersection. // TODO(turbofan): Compute the intersection.
} }
state = state->AddMaps(object, maps, zone()); state = state->SetMaps(object, maps, zone());
return UpdateState(node, state); return UpdateState(node, state);
} }


Expand All @@ -737,10 +761,9 @@ Reduction LoadElimination::ReduceCheckMaps(Node* node) {
ZoneHandleSet<Map> object_maps; ZoneHandleSet<Map> object_maps;
if (state->LookupMaps(object, &object_maps)) { if (state->LookupMaps(object, &object_maps)) {
if (maps.contains(object_maps)) return Replace(effect); if (maps.contains(object_maps)) return Replace(effect);
state = state->KillMaps(object, zone());
// TODO(turbofan): Compute the intersection. // TODO(turbofan): Compute the intersection.
} }
state = state->AddMaps(object, maps, zone()); state = state->SetMaps(object, maps, zone());
return UpdateState(node, state); return UpdateState(node, state);
} }


Expand Down Expand Up @@ -777,7 +800,7 @@ Reduction LoadElimination::ReduceEnsureWritableFastElements(Node* node) {
return Replace(elements); return Replace(elements);
} }
// We know that the resulting elements have the fixed array map. // We know that the resulting elements have the fixed array map.
state = state->AddMaps(node, fixed_array_maps, zone()); state = state->SetMaps(node, fixed_array_maps, zone());
// Kill the previous elements on {object}. // Kill the previous elements on {object}.
state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset), state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset),
MaybeHandle<Name>(), zone()); MaybeHandle<Name>(), zone());
Expand All @@ -795,11 +818,11 @@ Reduction LoadElimination::ReduceMaybeGrowFastElements(Node* node) {
if (state == nullptr) return NoChange(); if (state == nullptr) return NoChange();
if (mode == GrowFastElementsMode::kDoubleElements) { if (mode == GrowFastElementsMode::kDoubleElements) {
// We know that the resulting elements have the fixed double array map. // We know that the resulting elements have the fixed double array map.
state = state->AddMaps( state = state->SetMaps(
node, ZoneHandleSet<Map>(factory()->fixed_double_array_map()), zone()); node, ZoneHandleSet<Map>(factory()->fixed_double_array_map()), zone());
} else { } else {
// We know that the resulting elements have the fixed array map. // We know that the resulting elements have the fixed array map.
state = state->AddMaps( state = state->SetMaps(
node, ZoneHandleSet<Map>(factory()->fixed_array_map()), zone()); node, ZoneHandleSet<Map>(factory()->fixed_array_map()), zone());
} }
// Kill the previous elements on {object}. // Kill the previous elements on {object}.
Expand Down Expand Up @@ -840,9 +863,7 @@ Reduction LoadElimination::ReduceTransitionElementsKind(Node* node) {
if (object_maps.contains(ZoneHandleSet<Map>(source_map))) { if (object_maps.contains(ZoneHandleSet<Map>(source_map))) {
object_maps.remove(source_map, zone()); object_maps.remove(source_map, zone());
object_maps.insert(target_map, zone()); object_maps.insert(target_map, zone());
AliasStateInfo alias_info(state, object, source_map); state = state->SetMaps(object, object_maps, zone());
state = state->KillMaps(alias_info, zone());
state = state->AddMaps(object, object_maps, zone());
} }
} else { } else {
AliasStateInfo alias_info(state, object, source_map); AliasStateInfo alias_info(state, object, source_map);
Expand All @@ -866,8 +887,7 @@ Reduction LoadElimination::ReduceTransitionAndStoreElement(Node* node) {
if (state->LookupMaps(object, &object_maps)) { if (state->LookupMaps(object, &object_maps)) {
object_maps.insert(double_map, zone()); object_maps.insert(double_map, zone());
object_maps.insert(fast_map, zone()); object_maps.insert(fast_map, zone());
state = state->KillMaps(object, zone()); state = state->SetMaps(object, object_maps, zone());
state = state->AddMaps(object, object_maps, zone());
} }
// Kill the elements as well. // Kill the elements as well.
state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset), state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset),
Expand Down Expand Up @@ -911,7 +931,7 @@ Reduction LoadElimination::ReduceLoadField(Node* node) {
} }
Handle<Map> field_map; Handle<Map> field_map;
if (access.map.ToHandle(&field_map)) { if (access.map.ToHandle(&field_map)) {
state = state->AddMaps(node, ZoneHandleSet<Map>(field_map), zone()); state = state->SetMaps(node, ZoneHandleSet<Map>(field_map), zone());
} }
return UpdateState(node, state); return UpdateState(node, state);
} }
Expand All @@ -933,7 +953,7 @@ Reduction LoadElimination::ReduceStoreField(Node* node) {
// Record the new {object} map information. // Record the new {object} map information.
ZoneHandleSet<Map> object_maps( ZoneHandleSet<Map> object_maps(
bit_cast<Handle<Map>>(new_value_type->AsHeapConstant()->Value())); bit_cast<Handle<Map>>(new_value_type->AsHeapConstant()->Value()));
state = state->AddMaps(object, object_maps, zone()); state = state->SetMaps(object, object_maps, zone());
} }
} else { } else {
int field_index = FieldIndexOf(access); int field_index = FieldIndexOf(access);
Expand Down Expand Up @@ -1067,7 +1087,7 @@ LoadElimination::AbstractState const* LoadElimination::UpdateStateForPhi(
if (!input_state->LookupMaps(phi->InputAt(i), &input_maps)) return state; if (!input_state->LookupMaps(phi->InputAt(i), &input_maps)) return state;
if (input_maps != object_maps) return state; if (input_maps != object_maps) return state;
} }
return state->AddMaps(phi, object_maps, zone()); return state->SetMaps(phi, object_maps, zone());
} }


Reduction LoadElimination::ReduceEffectPhi(Node* node) { Reduction LoadElimination::ReduceEffectPhi(Node* node) {
Expand Down
9 changes: 3 additions & 6 deletions src/compiler/load-elimination.h
View file
Expand Up @@ -192,11 +192,8 @@ class V8_EXPORT_PRIVATE LoadElimination final
// effect paths through the graph. // effect paths through the graph.
class AbstractMaps final : public ZoneObject { class AbstractMaps final : public ZoneObject {
public: public:
explicit AbstractMaps(Zone* zone) : info_for_node_(zone) {} explicit AbstractMaps(Zone* zone);
AbstractMaps(Node* object, ZoneHandleSet<Map> maps, Zone* zone) AbstractMaps(Node* object, ZoneHandleSet<Map> maps, Zone* zone);
: info_for_node_(zone) {
info_for_node_.insert(std::make_pair(object, maps));
}


AbstractMaps const* Extend(Node* object, ZoneHandleSet<Map> maps, AbstractMaps const* Extend(Node* object, ZoneHandleSet<Map> maps,
Zone* zone) const; Zone* zone) const;
Expand Down Expand Up @@ -225,7 +222,7 @@ class V8_EXPORT_PRIVATE LoadElimination final
bool Equals(AbstractState const* that) const; bool Equals(AbstractState const* that) const;
void Merge(AbstractState const* that, Zone* zone); void Merge(AbstractState const* that, Zone* zone);


AbstractState const* AddMaps(Node* object, ZoneHandleSet<Map> maps, AbstractState const* SetMaps(Node* object, ZoneHandleSet<Map> maps,
Zone* zone) const; Zone* zone) const;
AbstractState const* KillMaps(Node* object, Zone* zone) const; AbstractState const* KillMaps(Node* object, Zone* zone) const;
AbstractState const* KillMaps(const AliasStateInfo& alias_info, AbstractState const* KillMaps(const AliasStateInfo& alias_info,
Expand Down

0 comments on commit 9884bc5

Please sign in to comment.