Commit ea891e87 authored by Jaroslav Sevcik's avatar Jaroslav Sevcik Committed by Commit Bot

[turbofan] Load elimination - use maps to avoid state invalidation.

Currently, the transition elements kind operation invalidates the
elements of all other arrays. In particular, if we loop over matrix 
elements using two nested loops, and do possibly transitioning access 
a[i][j] in every iteration, we invalidate the load elimination state 
for the array 'a' because a[i][j] might transition elements kind 
for the 'a[i]' array. As a result, the 'a[i]' access cannot be 
hoisted from the inner loop.

With this CL, we figure out that transitioning 'a[i]' cannot affect 'a'
because we know that 'a' does not have the transition's source map.

In the micro-benchmark below, the time for summing up the elements of
100x100 matrix improves by factor of 1.7 (from ~340ms to ~190ms).

function matrixSum(a) {
  let s = 0;
  let rowCount = a.length;
  let columnCount = a[0].length;
  for (let i = 0; i < rowCount; i++) {
    for (let j = 0; j < columnCount ; j++) {
      s += a[i][j];
    }
  }
  return s;
}

// Setup the matrices:
// Smi elements.
var ma = [[1, 2], [3, 4]];
// Holey double elements.
var b = Array(100);
for (let i = 0; i < 100; i++) b[i] = 1.1;
var mb = [];
for (let i = 0; i < 100; i++) mb[i] = b;

// Warmup.
matrixSum(mb);
matrixSum(mb);
matrixSum(ma);
matrixSum(mb);
matrixSum(ma);

%OptimizeFunctionOnNextCall(matrixSum);

function runner(m) {
  let s = 0;
  for (let i = 0; i < 1e4; i++) {
    s += matrixSum(m);
  }
  return s;
}
// Make sure the runner does not know the elements kinds.
%NeverOptimizeFunction(runner);

// Measure.
console.time("Sum");
runner(mb);
console.timeEnd("Sum");


Bug: v8:6344
Change-Id: Ie0642d8693ed63116b1aaed7d2f261fcb64729fe
Reviewed-on: https://chromium-review.googlesource.com/704634
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48334}
parent afbfddd7
......@@ -330,13 +330,29 @@ bool MayAlias(MaybeHandle<Name> x, MaybeHandle<Name> y) {
} // namespace
class LoadElimination::AliasStateInfo {
public:
AliasStateInfo(const AbstractState* state, Node* object, Handle<Map> map)
: state_(state), object_(object), map_(map) {}
AliasStateInfo(const AbstractState* state, Node* object)
: state_(state), object_(object) {}
bool MayAlias(Node* other) const;
private:
const AbstractState* state_;
Node* object_;
MaybeHandle<Map> map_;
};
LoadElimination::AbstractField const* LoadElimination::AbstractField::Kill(
Node* object, MaybeHandle<Name> name, Zone* zone) const {
const AliasStateInfo& alias_info, MaybeHandle<Name> name,
Zone* zone) const {
for (auto pair : this->info_for_node_) {
if (MayAlias(object, pair.first)) {
if (alias_info.MayAlias(pair.first)) {
AbstractField* that = new (zone) AbstractField(zone);
for (auto pair : this->info_for_node_) {
if (!MayAlias(object, pair.first) ||
if (!alias_info.MayAlias(pair.first) ||
!MayAlias(name, pair.second.name)) {
that->info_for_node_.insert(pair);
}
......@@ -367,12 +383,12 @@ bool LoadElimination::AbstractMaps::Lookup(
}
LoadElimination::AbstractMaps const* LoadElimination::AbstractMaps::Kill(
Node* object, Zone* zone) const {
const AliasStateInfo& alias_info, Zone* zone) const {
for (auto pair : this->info_for_node_) {
if (MayAlias(object, pair.first)) {
if (alias_info.MayAlias(pair.first)) {
AbstractMaps* that = new (zone) AbstractMaps(zone);
for (auto pair : this->info_for_node_) {
if (!MayAlias(object, pair.first)) that->info_for_node_.insert(pair);
if (!alias_info.MayAlias(pair.first)) that->info_for_node_.insert(pair);
}
return that;
}
......@@ -512,9 +528,9 @@ LoadElimination::AbstractState const* LoadElimination::AbstractState::AddMaps(
}
LoadElimination::AbstractState const* LoadElimination::AbstractState::KillMaps(
Node* object, Zone* zone) const {
const AliasStateInfo& alias_info, Zone* zone) const {
if (this->maps_) {
AbstractMaps const* that_maps = this->maps_->Kill(object, zone);
AbstractMaps const* that_maps = this->maps_->Kill(alias_info, zone);
if (this->maps_ != that_maps) {
AbstractState* that = new (zone) AbstractState(*this);
that->maps_ = that_maps;
......@@ -524,6 +540,12 @@ LoadElimination::AbstractState const* LoadElimination::AbstractState::KillMaps(
return this;
}
LoadElimination::AbstractState const* LoadElimination::AbstractState::KillMaps(
Node* object, Zone* zone) const {
AliasStateInfo alias_info(this, object);
return KillMaps(alias_info, zone);
}
Node* LoadElimination::AbstractState::LookupElement(
Node* object, Node* index, MachineRepresentation representation) const {
if (this->elements_) {
......@@ -578,8 +600,15 @@ LoadElimination::AbstractState const* LoadElimination::AbstractState::AddField(
LoadElimination::AbstractState const* LoadElimination::AbstractState::KillField(
Node* object, size_t index, MaybeHandle<Name> name, Zone* zone) const {
AliasStateInfo alias_info(this, object);
return KillField(alias_info, index, name, zone);
}
LoadElimination::AbstractState const* LoadElimination::AbstractState::KillField(
const AliasStateInfo& alias_info, size_t index, MaybeHandle<Name> name,
Zone* zone) const {
if (AbstractField const* this_field = this->fields_[index]) {
this_field = this_field->Kill(object, name, zone);
this_field = this_field->Kill(alias_info, name, zone);
if (this->fields_[index] != this_field) {
AbstractState* that = new (zone) AbstractState(*this);
that->fields_[index] = this_field;
......@@ -592,16 +621,18 @@ LoadElimination::AbstractState const* LoadElimination::AbstractState::KillField(
LoadElimination::AbstractState const*
LoadElimination::AbstractState::KillFields(Node* object, MaybeHandle<Name> name,
Zone* zone) const {
AliasStateInfo alias_info(this, object);
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, name, zone);
AbstractField const* that_field =
this_field->Kill(alias_info, 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, name, zone);
that->fields_[i] = this->fields_[i]->Kill(alias_info, name, zone);
}
}
return that;
......@@ -618,6 +649,22 @@ Node* LoadElimination::AbstractState::LookupField(Node* object,
return nullptr;
}
bool LoadElimination::AliasStateInfo::MayAlias(Node* other) const {
if (QueryAlias(object_, other) == kNoAlias) {
return false;
}
Handle<Map> map;
if (map_.ToHandle(&map)) {
ZoneHandleSet<Map> other_maps;
if (state_->LookupMaps(other, &other_maps) && other_maps.size() == 1) {
if (map.address() != other_maps.at(0).address()) {
return false;
}
}
}
return true;
}
void LoadElimination::AbstractState::Print() const {
if (checks_) {
PrintF(" checks:\n");
......@@ -772,6 +819,17 @@ Reduction LoadElimination::ReduceTransitionElementsKind(Node* node) {
Node* const effect = NodeProperties::GetEffectInput(node);
AbstractState const* state = node_states_.Get(effect);
if (state == nullptr) return NoChange();
switch (transition.mode()) {
case ElementsTransition::kFastTransition:
break;
case ElementsTransition::kSlowTransition:
// Kill the elements as well.
AliasStateInfo alias_info(state, object, source_map);
state =
state->KillField(alias_info, FieldIndexOf(JSObject::kElementsOffset),
MaybeHandle<Name>(), zone());
break;
}
ZoneHandleSet<Map> object_maps;
if (state->LookupMaps(object, &object_maps)) {
if (ZoneHandleSet<Map>(target_map).contains(object_maps)) {
......@@ -782,20 +840,13 @@ Reduction LoadElimination::ReduceTransitionElementsKind(Node* node) {
if (object_maps.contains(ZoneHandleSet<Map>(source_map))) {
object_maps.remove(source_map, zone());
object_maps.insert(target_map, zone());
state = state->KillMaps(object, zone());
AliasStateInfo alias_info(state, object, source_map);
state = state->KillMaps(alias_info, zone());
state = state->AddMaps(object, object_maps, zone());
}
} else {
state = state->KillMaps(object, zone());
}
switch (transition.mode()) {
case ElementsTransition::kFastTransition:
break;
case ElementsTransition::kSlowTransition:
// Kill the elements as well.
state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset),
MaybeHandle<Name>(), zone());
break;
AliasStateInfo alias_info(state, object, source_map);
state = state->KillMaps(alias_info, zone());
}
return UpdateState(node, state);
}
......@@ -1104,6 +1155,11 @@ Reduction LoadElimination::UpdateState(Node* node, AbstractState const* state) {
LoadElimination::AbstractState const* LoadElimination::ComputeLoopState(
Node* node, AbstractState const* state) const {
Node* const control = NodeProperties::GetControlInput(node);
struct TransitionElementsKindInfo {
ElementsTransition transition;
Node* object;
};
ZoneVector<TransitionElementsKindInfo> element_transitions_(zone());
ZoneQueue<Node*> queue(zone());
ZoneSet<Node*> visited(zone());
visited.insert(node);
......@@ -1138,17 +1194,7 @@ LoadElimination::AbstractState const* LoadElimination::ComputeLoopState(
if (!state->LookupMaps(object, &object_maps) ||
!ZoneHandleSet<Map>(transition.target())
.contains(object_maps)) {
state = state->KillMaps(object, zone());
switch (transition.mode()) {
case ElementsTransition::kFastTransition:
break;
case ElementsTransition::kSlowTransition:
// Kill the elements as well.
state = state->KillField(
object, FieldIndexOf(JSObject::kElementsOffset),
MaybeHandle<Name>(), zone());
break;
}
element_transitions_.push_back({transition, object});
}
break;
}
......@@ -1198,6 +1244,48 @@ LoadElimination::AbstractState const* LoadElimination::ComputeLoopState(
}
}
}
// Finally, we apply the element transitions. For each transition, we will try
// to only invalidate information about nodes that can have the transition's
// source map. The trouble is that an object can be transitioned by some other
// transition to the source map. In that case, the other transition will
// invalidate the information, so we are mostly fine.
//
// The only bad case is
//
// mapA ---fast---> mapB ---slow---> mapC
//
// If we process the slow transition first on an object that has mapA, we will
// ignore the transition because the object does not have its source map
// (mapB). When we later process the fast transition, we invalidate the
// object's map, but we keep the information about the object's elements. This
// is wrong because the elements will be overwritten by the slow transition.
//
// Note that the slow-slow case is fine because either of the slow transition
// will invalidate the elements field, so the processing order does not
// matter.
//
// To handle the bad case properly, we first kill the maps using all
// transitions. We kill the the fields later when all the transitions are
// already reflected in the map information.
for (const TransitionElementsKindInfo& t : element_transitions_) {
AliasStateInfo alias_info(state, t.object, t.transition.source());
state = state->KillMaps(alias_info, zone());
}
for (const TransitionElementsKindInfo& t : element_transitions_) {
switch (t.transition.mode()) {
case ElementsTransition::kFastTransition:
break;
case ElementsTransition::kSlowTransition: {
AliasStateInfo alias_info(state, t.object, t.transition.source());
state = state->KillField(alias_info,
FieldIndexOf(JSObject::kElementsOffset),
MaybeHandle<Name>(), zone());
break;
}
}
}
return state;
}
......
......@@ -125,6 +125,11 @@ class V8_EXPORT_PRIVATE LoadElimination final
size_t next_index_ = 0;
};
// Information we use to resolve object aliasing. Currently, we consider
// object not aliased if they have different maps or if the nodes may
// not alias.
class AliasStateInfo;
// Abstract state to approximate the current state of a certain field along
// the effect paths through the graph.
class AbstractField final : public ZoneObject {
......@@ -143,8 +148,8 @@ class V8_EXPORT_PRIVATE LoadElimination final
return that;
}
Node* Lookup(Node* object) const;
AbstractField const* Kill(Node* object, MaybeHandle<Name> name,
Zone* zone) const;
AbstractField const* Kill(const AliasStateInfo& alias_info,
MaybeHandle<Name> name, Zone* zone) const;
bool Equals(AbstractField const* that) const {
return this == that || this->info_for_node_ == that->info_for_node_;
}
......@@ -196,7 +201,8 @@ class V8_EXPORT_PRIVATE LoadElimination final
AbstractMaps const* Extend(Node* object, ZoneHandleSet<Map> maps,
Zone* zone) const;
bool Lookup(Node* object, ZoneHandleSet<Map>* object_maps) const;
AbstractMaps const* Kill(Node* object, Zone* zone) const;
AbstractMaps const* Kill(const AliasStateInfo& alias_info,
Zone* zone) const;
bool Equals(AbstractMaps const* that) const {
return this == that || this->info_for_node_ == that->info_for_node_;
}
......@@ -222,10 +228,15 @@ class V8_EXPORT_PRIVATE LoadElimination final
AbstractState const* AddMaps(Node* object, ZoneHandleSet<Map> maps,
Zone* zone) const;
AbstractState const* KillMaps(Node* object, Zone* zone) const;
AbstractState const* KillMaps(const AliasStateInfo& alias_info,
Zone* zone) const;
bool LookupMaps(Node* object, ZoneHandleSet<Map>* object_maps) const;
AbstractState const* AddField(Node* object, size_t index, Node* value,
MaybeHandle<Name> name, Zone* zone) const;
AbstractState const* KillField(const AliasStateInfo& alias_info,
size_t index, MaybeHandle<Name> name,
Zone* zone) const;
AbstractState const* KillField(Node* object, size_t index,
MaybeHandle<Name> name, Zone* zone) const;
AbstractState const* KillFields(Node* object, MaybeHandle<Name> name,
......
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