Commit b9a59e38 authored by Mythri A's avatar Mythri A Committed by Commit Bot

[turboprop] Don't consider stores to constant fields as mutable

Turboprop doesn't use optimizations based on field constness to reduce
the number of deoptimizations. While this is safe for loads, for stores
if a different value is stored to a const field we should update the
constness of the field. This is needed so we can safely deopt any other
code that is relying on the constness of the field. Currently, turboprop
doesn't do this. So for now treat stores to constant fields similar to
TurboFan. In future, we may consider adding code to update the field
constness if necessary to reduce the number of deoptimizations.


Bug: chromium:1172797, v8:9684
Change-Id: I1d660457cb5d647e1283a495040a7e452fe1ac7e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2673401
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Reviewed-by: 's avatarSathya Gunasekaran  <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72590}
parent 105ef0b3
...@@ -433,7 +433,8 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo( ...@@ -433,7 +433,8 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
PropertyConstness constness; PropertyConstness constness;
if (details.IsReadOnly() && !details.IsConfigurable()) { if (details.IsReadOnly() && !details.IsConfigurable()) {
constness = PropertyConstness::kConst; constness = PropertyConstness::kConst;
} else if (broker()->is_turboprop() && !map->is_prototype_map()) { } else if (broker()->is_turboprop() && !map->is_prototype_map() &&
!IsAnyStore(access_mode)) {
// The constness feedback is too unstable for the aggresive compilation // The constness feedback is too unstable for the aggresive compilation
// of turboprop. // of turboprop.
constness = PropertyConstness::kMutable; constness = PropertyConstness::kMutable;
......
...@@ -44,6 +44,10 @@ namespace compiler { ...@@ -44,6 +44,10 @@ namespace compiler {
// For a store during literal creation, do not walk up the prototype chain. // For a store during literal creation, do not walk up the prototype chain.
enum class AccessMode { kLoad, kStore, kStoreInLiteral, kHas }; enum class AccessMode { kLoad, kStore, kStoreInLiteral, kHas };
inline bool IsAnyStore(AccessMode mode) {
return mode == AccessMode::kStore || mode == AccessMode::kStoreInLiteral;
}
enum class SerializationPolicy { kAssumeSerialized, kSerializeIfNeeded }; enum class SerializationPolicy { kAssumeSerialized, kSerializeIfNeeded };
enum class OddballType : uint8_t { enum class OddballType : uint8_t {
......
...@@ -191,7 +191,7 @@ function TestStoreToConstantFieldOfConstantObject(the_value, other_value) { ...@@ -191,7 +191,7 @@ function TestStoreToConstantFieldOfConstantObject(the_value, other_value) {
assertOptimized(store); assertOptimized(store);
// Storing other value deoptimizes because of failed value check. // Storing other value deoptimizes because of failed value check.
store(other_value); store(other_value);
assertOptimized(store); assertUnoptimized(store);
assertEquals(other_value, constant_object.a.v); assertEquals(other_value, constant_object.a.v);
} }
......
// Copyright 2020 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 --turboprop --no-use-osr --opt --no-always-opt
var v_0 = {};
function f_0(o, v) {
o.f = v;
}
function f_1() {
return v_0.f;
}
%PrepareFunctionForOptimization(f_0);
f_0(v_0, 42);
f_0(v_0, 42);
%OptimizeFunctionOnNextCall(f_0);
f_0(v_0, 42);
// TP tier up
for (let i = 0; i < 100000; i++) {
f_1();
}
assertOptimized(f_0);
// TODO(mythria): Add an option to assert on the optimization tier and assert
// f_1 is optimized with TurboFan.
assertOptimized(f_1);
// Store in f_0 should trigger a change to the constness of the field.
f_0(v_0, 53);
// f_0 does a eager deopt and lets the interpreter update the field constness.
assertUnoptimized(f_0);
assertEquals(v_0.f, 53);
assertEquals(f_1(), 53);
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