[maglev] Record phi use as smi when eliding a write barrier

victorgomes · V8 LUCI CQ · commit 165dd8788846 · 2024-09-02T15:12:19.000Z
We can only elide the write barrier under the assumption that the phi's type will remain a Smi. If we don't record its tagged usage at this bytecode offset, the phi representation selector might later change its representation to int32. It will then emit a `Int32ToNumber` before `StoreTaggedFieldNoWriteBarrier`. This is incorrect because it could allocate a heap number, potentially promoting it to old space during a GC and requiring a write barrier. Fixed: 362784006 Change-Id: I775d7cc151189c12f59fc5ff0edccada1665f230 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5830348 Auto-Submit: Victor Gomes <victorgomes@chromium.org> Reviewed-by: Patrick Thier <pthier@chromium.org> Commit-Queue: Victor Gomes <victorgomes@chromium.org> Commit-Queue: Patrick Thier <pthier@chromium.org> Cr-Commit-Position: refs/heads/main@{#95916}
diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc
@@ -4307,7 +4307,10 @@ AllocationBlock* GetAllocation(ValueNode* object) {
 bool MaglevGraphBuilder::CanElideWriteBarrier(ValueNode* object,
                                               ValueNode* value) {
   if (value->Is<RootConstant>()) return true;
-  if (CheckType(value, NodeType::kSmi)) return true;
+  if (CheckType(value, NodeType::kSmi)) {
+    RecordUseReprHintIfPhi(value, UseRepresentation::kTagged);
+    return true;
+  }
 
   // No need for a write barrier if both object and value are part of the same
   // folded young allocation.
diff --git a/test/mjsunit/maglev/regress-362784006.js b/test/mjsunit/maglev/regress-362784006.js
@@ -0,0 +1,19 @@
+// Copyright 2024 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: --invocation-count-for-maglev-osr=1 --invocation-count-for-maglev=1
+// Flags: --minimum-invocations-after-ic-update=1 --deopt-every-n-times=250
+// Flags: --single-threaded --verify-heap
+
+// The flags ensure that we verify the heap just before we deopt the first
+// Maglev OSR synchronously.
+// --deopt-every-n-times > 0 forces MaterializeHeapObjects to call GC.
+
+let v0 = 1;
+let vn = 1000;
+for (let i1 = v0; i1 <= vn; i1 += v0) {
+    v0 = v0 && i1;
+    let v8 = "";
+    [v8,] = v8;
+}
