Commit d871c5ba authored by Tobias Tebbi's avatar Tobias Tebbi Committed by Commit Bot

[turbofan] fix escape analysis divergence on Air benchmark

When a virtual object passes by a store node that updates a field to the existing value, then the object and its state were not copied, which lead to the original object being passed on. 
If then later the store actually modifies and copies the virtual object, this new copy is not passed down the effect chain, so subsequent nodes still refer to the original virtual object and try to update it once new information flows in.
This conflicts with updates on the node that originally created the virtual object, leading to divergence.

Bug: v8:6345
Change-Id: Iab1ce98a60b48478b343eae765c80bdfcb8ba390
Reviewed-on: https://chromium-review.googlesource.com/496267
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45120}
parent 2238a16c
......@@ -168,6 +168,8 @@ class VirtualObject : public ZoneObject {
bool IsCreatedPhi(size_t offset) { return phi_[offset]; }
void SetField(size_t offset, Node* node, bool created_phi = false) {
TRACE(" VirtualObject(%p)[%zu] changes from #%i to #%i\n", this, offset,
fields_[offset] ? fields_[offset]->id() : -1, node ? node->id() : -1);
fields_[offset] = node;
phi_[offset] = created_phi;
}
......@@ -234,6 +236,7 @@ class VirtualObject : public ZoneObject {
DEFINE_OPERATORS_FOR_FLAGS(VirtualObject::StatusFlags)
bool VirtualObject::UpdateFrom(const VirtualObject& other) {
TRACE("%p.UpdateFrom(%p)\n", this, &other);
bool changed = status_ != other.status_;
status_ = other.status_;
phi_ = other.phi_;
......@@ -1262,6 +1265,10 @@ void EscapeAnalysis::ForwardVirtualState(Node* node) {
Node* effect = NodeProperties::GetEffectInput(node);
DCHECK_NOT_NULL(virtual_states_[effect->id()]);
if (virtual_states_[node->id()]) {
TRACE("Updating virtual state %p at %s#%d from virtual state %p at %s#%d\n",
virtual_states_[node->id()], node->op()->mnemonic(), node->id(),
virtual_states_[effect->id()], effect->op()->mnemonic(),
effect->id());
virtual_states_[node->id()]->UpdateFrom(virtual_states_[effect->id()],
zone());
} else {
......@@ -1663,8 +1670,8 @@ void EscapeAnalysis::ProcessStoreField(Node* node) {
FieldAccessOf(node->op()).offset == Name::kHashFieldOffset);
val = slot_not_analyzed_;
}
object = CopyForModificationAt(object, state, node);
if (object->GetField(offset) != val) {
object = CopyForModificationAt(object, state, node);
object->SetField(offset, val);
}
}
......@@ -1687,8 +1694,8 @@ void EscapeAnalysis::ProcessStoreElement(Node* node) {
int offset = OffsetForElementAccess(node, index.Value());
if (static_cast<size_t>(offset) >= object->field_count()) return;
Node* val = ResolveReplacement(NodeProperties::GetValueInput(node, 2));
object = CopyForModificationAt(object, state, node);
if (object->GetField(offset) != val) {
object = CopyForModificationAt(object, state, node);
object->SetField(offset, val);
}
}
......@@ -1703,8 +1710,8 @@ void EscapeAnalysis::ProcessStoreElement(Node* node) {
}
if (VirtualObject* object = GetVirtualObject(state, to)) {
if (!object->IsTracked()) return;
object = CopyForModificationAt(object, state, node);
if (!object->AllFieldsClear()) {
object = CopyForModificationAt(object, state, node);
object->ClearAllFields();
TRACE("Cleared all fields of @%d:#%d\n",
status_analysis_->GetAlias(object->id()), object->id());
......
// Copyright 2017 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax --no-turbo-loop-peeling --turbo-escape
function foo(){
var o = {a : 5}
for (var i = 0; i < 100; ++i) {
o.a = 5;
o.a = 7;
}
}
foo();
foo();
%OptimizeFunctionOnNextCall(foo)
foo();
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