[runtime][compiler] Be less strict about PropertyCell changes

Don't deopt when a PropertyCell changes from readonly to writable.
Turbofan doesn't depend on readonly-ness unless the property is
also non-configurable, in which case such a change can't happen.

Change-Id: I3d1078a8adf1ec1b16d973dd71c4295d71003a8d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2531791
Commit-Queue: Georg Neis <neis@chromium.org>
Auto-Submit: Georg Neis <neis@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71184}

src/objects/dictionary-inl.h

@@ -321,7 +321,11 @@ void GlobalDictionaryShape::DetailsAtPut(Dictionary dict, InternalIndex entry,
                                          PropertyDetails value) {
   DCHECK(entry.is_found());
   PropertyCell cell = dict.CellAt(entry);
-  if (cell.property_details().IsReadOnly() != value.IsReadOnly()) {
+  // Deopt when when making a writable property read-only. The reverse direction
+  // is uninteresting because Turbofan does not currently rely on read-only
+  // unless the property is also configurable, in which case it will stay
+  // read-only forever.
+  if (!cell.property_details().IsReadOnly() && value.IsReadOnly()) {
     cell.dependent_code().DeoptimizeDependentCodeGroup(
         DependentCode::kPropertyCellChangedGroup);
   }

src/objects/objects.cc

@@ -6444,9 +6444,14 @@ Handle<PropertyCell> PropertyCell::PrepareForValue(
     cell->set_value(*value);
   }
 
-  // Deopt when transitioning from a constant type.
-  if (!invalidate && (original_details.cell_type() != new_type ||
-                      original_details.IsReadOnly() != details.IsReadOnly())) {
+  // Deopt when transitioning from a constant type or when making a writable
+  // property read-only. Making a read-only property writable again is not
+  // interesting because Turbofan does not currently rely on read-only unless
+  // the property is also configurable, in which case it will stay read-only
+  // forever.
+  if (!invalidate &&
+      (original_details.cell_type() != new_type ||
+       (!original_details.IsReadOnly() && details.IsReadOnly()))) {
     cell->dependent_code().DeoptimizeDependentCodeGroup(
         DependentCode::kPropertyCellChangedGroup);
   }

test/mjsunit/compiler/globals-freeze-constant.js

+// 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 --opt --no-always-opt
+
+
+glo = 0;
+
+function write_glo(x) { glo = x }
+
+%PrepareFunctionForOptimization(write_glo);
+write_glo(0);
+assertEquals(0, glo);
+
+// At this point, glo has cell type Constant.
+
+%OptimizeFunctionOnNextCall(write_glo);
+write_glo(0);
+assertEquals(0, glo);
+assertOptimized(write_glo);
+
+Object.freeze(this);
+assertUnoptimized(write_glo);
+write_glo(1);
+assertEquals(0, glo);

test/mjsunit/compiler/globals-freeze-constanttype.js

+// 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 --opt --no-always-opt
+
+
+glo = 0;
+
+function write_glo(x) { glo = x }
+
+%PrepareFunctionForOptimization(write_glo);
+write_glo(1);
+assertEquals(1, glo);
+
+// At this point, glo has cell type ConstantType.
+
+%OptimizeFunctionOnNextCall(write_glo);
+write_glo(0);
+assertEquals(0, glo);
+assertOptimized(write_glo);
+
+Object.freeze(this);
+assertUnoptimized(write_glo);
+write_glo(1);
+assertEquals(0, glo);

test/mjsunit/compiler/globals-freeze-mutable.js

+// 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 --opt --no-always-opt
+
+
+glo = 0;
+
+function write_glo(x) { glo = x }
+
+%PrepareFunctionForOptimization(write_glo);
+write_glo({});
+write_glo(1);
+assertEquals(1, glo);
+
+// At this point, glo has cell type Mutable.
+
+%OptimizeFunctionOnNextCall(write_glo);
+write_glo(0);
+assertEquals(0, glo);
+assertOptimized(write_glo);
+
+Object.freeze(this);
+assertUnoptimized(write_glo);
+write_glo(1);
+assertEquals(0, glo);

test/mjsunit/compiler/globals.js

@@ -25,7 +25,10 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+// Flags: --allow-natives-syntax --opt --no-always-opt
+
 // Test references and assignments to global variables.
+
 var g = 0;
 
 // Test compilation of a global variable store.
@@ -74,3 +77,144 @@ glo2 = 0;
 function f2(x) { glo2 = x; }
 f2(42);
 assertEquals(42, glo2);
+
+
+//////////////////////////////////////////////////////////////////////////////
+
+
+// Test Constant cell value going from writable to read-only.
+{
+  glo4 = 4;
+
+  function write_glo4(x) { glo4 = x }
+
+  %PrepareFunctionForOptimization(write_glo4);
+  write_glo4(4);
+  assertEquals(4, glo4);
+
+  // At this point, glo4 has cell type Constant.
+
+  %OptimizeFunctionOnNextCall(write_glo4);
+  write_glo4(4);
+  assertEquals(4, glo4);
+  assertOptimized(write_glo4);
+
+  Object.defineProperty(this, 'glo4', {writable: false});
+  assertUnoptimized(write_glo4);
+  write_glo4(2);
+  assertEquals(4, glo4);
+}
+
+
+// Test ConstantType cell value going from writable to read-only.
+{
+  glo5 = 5;
+
+  function write_glo5(x) { glo5 = x }
+
+  %PrepareFunctionForOptimization(write_glo5);
+  write_glo5(0);
+  assertEquals(0, glo5);
+
+  // At this point, glo5 has cell type ConstantType.
+
+  %OptimizeFunctionOnNextCall(write_glo5);
+  write_glo5(5);
+  assertEquals(5, glo5);
+  assertOptimized(write_glo5);
+
+  Object.defineProperty(this, 'glo5', {writable: false});
+  assertUnoptimized(write_glo5);
+  write_glo5(2);
+  assertEquals(5, glo5);
+}
+
+
+// Test Mutable cell value going from writable to read-only.
+{
+  glo6 = 6;
+
+  function write_glo6(x) { glo6 = x }
+
+  %PrepareFunctionForOptimization(write_glo6);
+  write_glo6({});
+  write_glo6(3);
+  assertEquals(3, glo6);
+
+  // At this point, glo6 has cell type Mutable.
+
+  %OptimizeFunctionOnNextCall(write_glo6);
+  write_glo6(6);
+  assertEquals(6, glo6);
+  assertOptimized(write_glo6);
+
+  Object.defineProperty(this, 'glo6', {writable: false});
+  assertUnoptimized(write_glo6);
+  write_glo6(2);
+  assertEquals(6, glo6);
+}
+
+
+// Test Constant cell value going from read-only to writable.
+{
+  glo7 = 7;
+  Object.defineProperty(this, 'glo7', {writable: false});
+
+  function read_glo7() { return glo7 }
+
+  %PrepareFunctionForOptimization(read_glo7);
+  assertEquals(7, read_glo7());
+
+  // At this point, glo7 has cell type Constant.
+
+  %OptimizeFunctionOnNextCall(read_glo7);
+  assertEquals(7, read_glo7());
+
+  Object.defineProperty(this, 'glo7', {writable: true});
+  assertEquals(7, read_glo7());
+  assertOptimized(read_glo7);
+}
+
+
+// Test ConstantType cell value going from read-only to writable.
+{
+  glo8 = 0;
+  glo8 = 8;
+  Object.defineProperty(this, 'glo8', {writable: false});
+
+  function read_glo8() { return glo8 }
+
+  %PrepareFunctionForOptimization(read_glo8);
+  assertEquals(8, read_glo8());
+
+  // At this point, glo8 has cell type ConstantType.
+
+  %OptimizeFunctionOnNextCall(read_glo8);
+  assertEquals(8, read_glo8());
+
+  Object.defineProperty(this, 'glo8', {writable: true});
+  assertEquals(8, read_glo8());
+  assertOptimized(read_glo8);
+}
+
+
+// Test Mutable cell value going from read-only to writable.
+{
+  glo9 = {};
+  glo9 = 9;
+  Object.defineProperty(this, 'glo9', {writable: false});
+
+  function read_glo9() { return glo9 }
+
+  %PrepareFunctionForOptimization(read_glo9);
+  assertEquals(9, read_glo9());
+
+  // At this point, glo9 has cell type Mutable.
+
+  %OptimizeFunctionOnNextCall(read_glo9);
+  assertEquals(9, read_glo9());
+
+  Object.defineProperty(this, 'glo9', {writable: true});
+  assertEquals(9, read_glo9());
+  assertOptimized(read_glo9);
+}