From 4dd3d0610bab9f69c35b2892e6d25dc45b9adffb Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Fri, 18 Aug 2017 13:11:10 +0200 Subject: [PATCH] [turbofan] Consider field name for aliasing. In LoadElimination, don't consider two fields as potentially aliasing if they have different names. This gives another 5% boost on the Octane/DeltaBlue benchmark, since the redundant loads and checks on the elms of the OrderedCollection can be properly eliminated in the chainTest. Bug: v8:5267 Change-Id: Id2dbb8cac02f9c95a85e5cc8acac3f66b679fd06 Reviewed-on: https://chromium-review.googlesource.com/620727 Reviewed-by: Jaroslav Sevcik Commit-Queue: Benedikt Meurer Cr-Commit-Position: refs/heads/master@{#47424} --- src/compiler/load-elimination.cc | 102 +++++++++++++++++++------------ src/compiler/load-elimination.h | 37 +++++++---- 2 files changed, 89 insertions(+), 50 deletions(-) diff --git a/src/compiler/load-elimination.cc b/src/compiler/load-elimination.cc index ace91a6ef4..6402bd9694 100644 --- a/src/compiler/load-elimination.cc +++ b/src/compiler/load-elimination.cc @@ -312,18 +312,32 @@ void LoadElimination::AbstractElements::Print() const { Node* LoadElimination::AbstractField::Lookup(Node* object) const { for (auto pair : info_for_node_) { - if (MustAlias(object, pair.first)) return pair.second; + if (MustAlias(object, pair.first)) return pair.second.value; } return nullptr; } +namespace { + +bool MayAlias(MaybeHandle x, MaybeHandle y) { + if (!x.address()) return true; + if (!y.address()) return true; + if (x.address() != y.address()) return false; + return true; +} + +} // namespace + LoadElimination::AbstractField const* LoadElimination::AbstractField::Kill( - Node* object, Zone* zone) const { + Node* object, MaybeHandle name, Zone* zone) const { for (auto pair : this->info_for_node_) { if (MayAlias(object, pair.first)) { AbstractField* that = new (zone) AbstractField(zone); for (auto pair : this->info_for_node_) { - if (!MayAlias(object, pair.first)) that->info_for_node_.insert(pair); + if (!MayAlias(object, pair.first) || + !MayAlias(name, pair.second.name)) { + that->info_for_node_.insert(pair); + } } return that; } @@ -334,8 +348,8 @@ LoadElimination::AbstractField const* LoadElimination::AbstractField::Kill( void LoadElimination::AbstractField::Print() const { for (auto pair : info_for_node_) { PrintF(" #%d:%s -> #%d:%s\n", pair.first->id(), - pair.first->op()->mnemonic(), pair.second->id(), - pair.second->op()->mnemonic()); + pair.first->op()->mnemonic(), pair.second.value->id(), + pair.second.value->op()->mnemonic()); } } @@ -525,20 +539,22 @@ LoadElimination::AbstractState::KillElement(Node* object, Node* index, } LoadElimination::AbstractState const* LoadElimination::AbstractState::AddField( - Node* object, size_t index, Node* value, Zone* zone) const { + Node* object, size_t index, Node* value, MaybeHandle name, + Zone* zone) const { AbstractState* that = new (zone) AbstractState(*this); if (that->fields_[index]) { - that->fields_[index] = that->fields_[index]->Extend(object, value, zone); + that->fields_[index] = + that->fields_[index]->Extend(object, value, name, zone); } else { - that->fields_[index] = new (zone) AbstractField(object, value, zone); + that->fields_[index] = new (zone) AbstractField(object, value, name, zone); } return that; } LoadElimination::AbstractState const* LoadElimination::AbstractState::KillField( - Node* object, size_t index, Zone* zone) const { + Node* object, size_t index, MaybeHandle name, Zone* zone) const { if (AbstractField const* this_field = this->fields_[index]) { - this_field = this_field->Kill(object, zone); + this_field = this_field->Kill(object, name, zone); if (this->fields_[index] != this_field) { AbstractState* that = new (zone) AbstractState(*this); that->fields_[index] = this_field; @@ -549,17 +565,18 @@ LoadElimination::AbstractState const* LoadElimination::AbstractState::KillField( } LoadElimination::AbstractState const* -LoadElimination::AbstractState::KillFields(Node* object, Zone* zone) const { +LoadElimination::AbstractState::KillFields(Node* object, MaybeHandle name, + Zone* zone) const { for (size_t i = 0;; ++i) { if (i == arraysize(fields_)) return this; if (AbstractField const* this_field = this->fields_[i]) { - AbstractField const* that_field = this_field->Kill(object, zone); + AbstractField const* that_field = this_field->Kill(object, name, zone); if (that_field != this_field) { AbstractState* that = new (zone) AbstractState(*this); that->fields_[i] = that_field; while (++i < arraysize(fields_)) { if (this->fields_[i] != nullptr) { - that->fields_[i] = this->fields_[i]->Kill(object, zone); + that->fields_[i] = this->fields_[i]->Kill(object, name, zone); } } return that; @@ -672,11 +689,11 @@ Reduction LoadElimination::ReduceEnsureWritableFastElements(Node* node) { // We know that the resulting elements have the fixed array map. state = state->AddMaps(node, fixed_array_maps, zone()); // Kill the previous elements on {object}. - state = - state->KillField(object, FieldIndexOf(JSObject::kElementsOffset), zone()); + state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset), + MaybeHandle(), zone()); // Add the new elements on {object}. state = state->AddField(object, FieldIndexOf(JSObject::kElementsOffset), node, - zone()); + MaybeHandle(), zone()); return UpdateState(node, state); } @@ -697,15 +714,15 @@ Reduction LoadElimination::ReduceMaybeGrowFastElements(Node* node) { } if (flags & GrowFastElementsFlag::kArrayObject) { // Kill the previous Array::length on {object}. - state = - state->KillField(object, FieldIndexOf(JSArray::kLengthOffset), zone()); + state = state->KillField(object, FieldIndexOf(JSArray::kLengthOffset), + factory()->length_string(), zone()); } // Kill the previous elements on {object}. - state = - state->KillField(object, FieldIndexOf(JSObject::kElementsOffset), zone()); + state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset), + MaybeHandle(), zone()); // Add the new elements on {object}. state = state->AddField(object, FieldIndexOf(JSObject::kElementsOffset), node, - zone()); + MaybeHandle(), zone()); return UpdateState(node, state); } @@ -739,7 +756,7 @@ Reduction LoadElimination::ReduceTransitionElementsKind(Node* node) { case ElementsTransition::kSlowTransition: // Kill the elements as well. state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset), - zone()); + MaybeHandle(), zone()); break; } return UpdateState(node, state); @@ -764,8 +781,8 @@ Reduction LoadElimination::ReduceTransitionAndStoreElement(Node* node) { state = state->AddMaps(object, object_maps, zone()); } // Kill the elements as well. - state = - state->KillField(object, FieldIndexOf(JSObject::kElementsOffset), zone()); + state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset), + MaybeHandle(), zone()); return UpdateState(node, state); } @@ -800,7 +817,7 @@ Reduction LoadElimination::ReduceLoadField(Node* node) { return Replace(replacement); } } - state = state->AddField(object, field_index, node, zone()); + state = state->AddField(object, field_index, node, access.name, zone()); } } Handle field_map; @@ -838,11 +855,12 @@ Reduction LoadElimination::ReduceStoreField(Node* node) { return Replace(effect); } // Kill all potentially aliasing fields and record the new value. - state = state->KillField(object, field_index, zone()); - state = state->AddField(object, field_index, new_value, zone()); + state = state->KillField(object, field_index, access.name, zone()); + state = + state->AddField(object, field_index, new_value, access.name, zone()); } else { // Unsupported StoreField operator. - state = state->KillFields(object, zone()); + state = state->KillFields(object, access.name, zone()); } } return UpdateState(node, state); @@ -1032,19 +1050,22 @@ LoadElimination::AbstractState const* LoadElimination::ComputeLoopState( switch (current->opcode()) { case IrOpcode::kEnsureWritableFastElements: { Node* const object = NodeProperties::GetValueInput(current, 0); - state = state->KillField( - object, FieldIndexOf(JSObject::kElementsOffset), zone()); + state = state->KillField(object, + FieldIndexOf(JSObject::kElementsOffset), + MaybeHandle(), zone()); break; } case IrOpcode::kMaybeGrowFastElements: { GrowFastElementsFlags flags = GrowFastElementsFlagsOf(current->op()); Node* const object = NodeProperties::GetValueInput(current, 0); - state = state->KillField( - object, FieldIndexOf(JSObject::kElementsOffset), zone()); + state = state->KillField(object, + FieldIndexOf(JSObject::kElementsOffset), + MaybeHandle(), zone()); if (flags & GrowFastElementsFlag::kArrayObject) { - state = state->KillField( - object, FieldIndexOf(JSArray::kLengthOffset), zone()); + state = + state->KillField(object, FieldIndexOf(JSArray::kLengthOffset), + factory()->length_string(), zone()); } break; } @@ -1062,7 +1083,8 @@ LoadElimination::AbstractState const* LoadElimination::ComputeLoopState( case ElementsTransition::kSlowTransition: // Kill the elements as well. state = state->KillField( - object, FieldIndexOf(JSObject::kElementsOffset), zone()); + object, FieldIndexOf(JSObject::kElementsOffset), + MaybeHandle(), zone()); break; } } @@ -1073,8 +1095,9 @@ LoadElimination::AbstractState const* LoadElimination::ComputeLoopState( // Invalidate what we know about the {object}s map. state = state->KillMaps(object, zone()); // Kill the elements as well. - state = state->KillField( - object, FieldIndexOf(JSObject::kElementsOffset), zone()); + state = state->KillField(object, + FieldIndexOf(JSObject::kElementsOffset), + MaybeHandle(), zone()); break; } case IrOpcode::kStoreField: { @@ -1086,9 +1109,10 @@ LoadElimination::AbstractState const* LoadElimination::ComputeLoopState( } else { int field_index = FieldIndexOf(access); if (field_index < 0) { - state = state->KillFields(object, zone()); + state = state->KillFields(object, access.name, zone()); } else { - state = state->KillField(object, field_index, zone()); + state = + state->KillField(object, field_index, access.name, zone()); } } break; diff --git a/src/compiler/load-elimination.h b/src/compiler/load-elimination.h index 3fdb660a3c..12d97e081f 100644 --- a/src/compiler/load-elimination.h +++ b/src/compiler/load-elimination.h @@ -130,19 +130,21 @@ class V8_EXPORT_PRIVATE LoadElimination final class AbstractField final : public ZoneObject { public: explicit AbstractField(Zone* zone) : info_for_node_(zone) {} - AbstractField(Node* object, Node* value, Zone* zone) + AbstractField(Node* object, Node* value, MaybeHandle name, Zone* zone) : info_for_node_(zone) { - info_for_node_.insert(std::make_pair(object, value)); + info_for_node_.insert(std::make_pair(object, Field(value, name))); } - AbstractField const* Extend(Node* object, Node* value, Zone* zone) const { + AbstractField const* Extend(Node* object, Node* value, + MaybeHandle name, Zone* zone) const { AbstractField* that = new (zone) AbstractField(zone); that->info_for_node_ = this->info_for_node_; - that->info_for_node_.insert(std::make_pair(object, value)); + that->info_for_node_.insert(std::make_pair(object, Field(value, name))); return that; } Node* Lookup(Node* object) const; - AbstractField const* Kill(Node* object, Zone* zone) const; + AbstractField const* Kill(Node* object, MaybeHandle name, + Zone* zone) const; bool Equals(AbstractField const* that) const { return this == that || this->info_for_node_ == that->info_for_node_; } @@ -151,10 +153,10 @@ class V8_EXPORT_PRIVATE LoadElimination final AbstractField* copy = new (zone) AbstractField(zone); for (auto this_it : this->info_for_node_) { Node* this_object = this_it.first; - Node* this_value = this_it.second; + Field this_second = this_it.second; auto that_it = that->info_for_node_.find(this_object); if (that_it != that->info_for_node_.end() && - that_it->second == this_value) { + that_it->second == this_second) { copy->info_for_node_.insert(this_it); } } @@ -164,7 +166,19 @@ class V8_EXPORT_PRIVATE LoadElimination final void Print() const; private: - ZoneMap info_for_node_; + struct Field { + Field() {} + Field(Node* value, MaybeHandle name) : value(value), name(name) {} + + bool operator==(const Field& other) const { + return value == other.value && name.address() == other.name.address(); + } + + Node* value = nullptr; + MaybeHandle name; + }; + + ZoneMap info_for_node_; }; static size_t const kMaxTrackedFields = 32; @@ -229,10 +243,11 @@ class V8_EXPORT_PRIVATE LoadElimination final bool LookupMaps(Node* object, ZoneHandleSet* object_maps) const; AbstractState const* AddField(Node* object, size_t index, Node* value, - Zone* zone) const; + MaybeHandle name, Zone* zone) const; AbstractState const* KillField(Node* object, size_t index, - Zone* zone) const; - AbstractState const* KillFields(Node* object, Zone* zone) const; + MaybeHandle name, Zone* zone) const; + AbstractState const* KillFields(Node* object, MaybeHandle name, + Zone* zone) const; Node* LookupField(Node* object, size_t index) const; AbstractState const* AddElement(Node* object, Node* index, Node* value,