From 0fff08a529f436a8eedb8d1f1b4cc5bcee4bf670 Mon Sep 17 00:00:00 2001
From: Darius Mercadier <[email protected]>
Date: Wed, 11 Mar 2026 07:57:37 +0100
Subject: [PATCH] [maglev] Stop eliding Smi checks because of CheckValueEqualsInt32

If a Phi goes through a CheckValueEqualsInt32 then we record a
constant int32 alternative for this Phi.

If the Phi is then the input to a BuildCheckSmi, we'll update the KNA
to record that this Phi is a Smi but won't insert a Smi check if the
int32 constant alternative is in range. Notably, this doesn't call
UseRequiresSmi, which means that when materializing this Phi, we won't
really know that it needs to be a Smi rather than a HeapObject.

Where is goes wrong is if this phi has a use that requires it to be
materialized to HeapObject (this isn't easy to construct, cf the -2.js
repro in this CL: I used a secondary Phi to trigger this), then we'll
always materialize it as a HeapObject and never as a Smi, which can
be wrong if we store it in a Smi field.

Fixed: 490558172
Change-Id: Idcc7077187a3e952f7d3eb42150525d33cefe430
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7653635
Auto-Submit: Darius Mercadier <[email protected]>
Reviewed-by: Victor Gomes <[email protected]>
Commit-Queue: Victor Gomes <[email protected]>
Cr-Commit-Position: refs/heads/main@{#105717}
---

diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc
index 9f8d4c9..6dfedbd 100644
--- a/src/maglev/maglev-graph-builder.cc
+++ b/src/maglev/maglev-graph-builder.cc
@@ -4183,6 +4183,7 @@
     return EmitUnconditionalDeopt(DeoptimizeReason::kSmi);
   }
   if (EnsureType(object, NodeType::kSmi) && elidable) return object;
+  RecordSmiUse(object);
   // For constants, we may be able to skip the runtime check.
   if (std::optional<int32_t> constant_value = TryGetInt32Constant(object)) {
     if (Smi::IsValid(constant_value.value())) return object;
@@ -4203,7 +4204,6 @@
       AddNewNodeNoInputConversion<CheckHoleyFloat64IsSmi>({object});
       break;
     case ValueRepresentation::kTagged:
-      RecordSmiUse(object);
       AddNewNodeNoInputConversion<CheckSmi>({object});
       break;
     case ValueRepresentation::kIntPtr:
diff --git a/test/mjsunit/maglev/regress-490558172-1.js b/test/mjsunit/maglev/regress-490558172-1.js
new file mode 100644
index 0000000..6f5fd14
--- /dev/null
+++ b/test/mjsunit/maglev/regress-490558172-1.js
@@ -0,0 +1,32 @@
+// Copyright 2026 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 --verify-heap
+
+let cst = 11;
+
+let o = { x : 17 };
+o.x = 25;
+
+function foo(c) {
+  for (let i = 0; i < 42; i++) {
+    if (i == 11) {
+      cst = i; // Will insert a CheckValueEqualsInt32
+      o.x = i; // Will not insert a CheckSmi because CheckValueEqualsInt32
+               // registers an Smi-sized Int32 constant alternative.
+    }
+  }
+}
+
+%PrepareFunctionForOptimization(foo);
+foo(true);
+foo(true);
+
+o.x = 42;
+
+%OptimizeMaglevOnNextCall(foo);
+foo(true);
+
+assertEquals(11, cst);
+assertEquals(11, o.x);
diff --git a/test/mjsunit/maglev/regress-490558172-2.js b/test/mjsunit/maglev/regress-490558172-2.js
new file mode 100644
index 0000000..f6fdb91
--- /dev/null
+++ b/test/mjsunit/maglev/regress-490558172-2.js
@@ -0,0 +1,49 @@
+// Copyright 2026 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 --verify-heap --maglev
+
+const non_smi_int32 = 0x40000000;
+let cst = 47;
+
+let o1 = { x : 17 };
+o1.x = 25;
+
+let o2 = { y : "abc" };
+o2.y = [];
+
+function foo(c) {
+  let phi = 42;
+  for (let i = 0; i < 10; i++) {
+    if (i == 5) {
+      cst = phi;  // Will insert a CheckValueEqualsInt32
+      // Requires Smi.
+      o1.x = phi; // Will not insert a CheckSmi because CheckValueEqualsInt32
+                  // registers an Smi-sized Int32 constant alternative.
+
+    }
+    phi++;
+  }
+  let phi2 = c ? phi : non_smi_int32;
+  // Requires HeapNumber
+  o2.y = phi2;
+}
+
+%PrepareFunctionForOptimization(foo);
+foo(false);
+foo(false);
+
+%OptimizeMaglevOnNextCall(foo);
+foo(false);
+assertOptimized(foo);
+
+assertEquals(47, cst);
+assertEquals(47, o1.x);
+assertEquals(non_smi_int32, o2.y);
+
+foo(true);
+assertUnoptimized(foo);
+assertEquals(47, cst);
+assertEquals(47, o1.x);
+assertEquals(52, o2.y);

diff --git a/test/mjsunit/maglev/regress-490558172-1.js b/test/mjsunit/maglev/regress-490558172-1.js
new file mode 100644
index 0000000..6f5fd14
--- /dev/null
+++ b/test/mjsunit/maglev/regress-490558172-1.js
@@ -0,0 +1,32 @@
+// Copyright 2026 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 --verify-heap
+
+let cst = 11;
+
+let o = { x : 17 };
+o.x = 25;
+
+function foo(c) {
+  for (let i = 0; i < 42; i++) {
+    if (i == 11) {
+      cst = i; // Will insert a CheckValueEqualsInt32
+      o.x = i; // Will not insert a CheckSmi because CheckValueEqualsInt32
+               // registers an Smi-sized Int32 constant alternative.
+    }
+  }
+}
+
+%PrepareFunctionForOptimization(foo);
+foo(true);
+foo(true);
+
+o.x = 42;
+
+%OptimizeMaglevOnNextCall(foo);
+foo(true);
+
+assertEquals(11, cst);
+assertEquals(11, o.x);
diff --git a/test/mjsunit/maglev/regress-490558172-2.js b/test/mjsunit/maglev/regress-490558172-2.js
new file mode 100644
index 0000000..f6fdb91
--- /dev/null
+++ b/test/mjsunit/maglev/regress-490558172-2.js
@@ -0,0 +1,49 @@
+// Copyright 2026 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 --verify-heap --maglev
+
+const non_smi_int32 = 0x40000000;
+let cst = 47;
+
+let o1 = { x : 17 };
+o1.x = 25;
+
+let o2 = { y : "abc" };
+o2.y = [];
+
+function foo(c) {
+  let phi = 42;
+  for (let i = 0; i < 10; i++) {
+    if (i == 5) {
+      cst = phi;  // Will insert a CheckValueEqualsInt32
+      // Requires Smi.
+      o1.x = phi; // Will not insert a CheckSmi because CheckValueEqualsInt32
+                  // registers an Smi-sized Int32 constant alternative.
+
+    }
+    phi++;
+  }
+  let phi2 = c ? phi : non_smi_int32;
+  // Requires HeapNumber
+  o2.y = phi2;
+}
+
+%PrepareFunctionForOptimization(foo);
+foo(false);
+foo(false);
+
+%OptimizeMaglevOnNextCall(foo);
+foo(false);
+assertOptimized(foo);
+
+assertEquals(47, cst);
+assertEquals(47, o1.x);
+assertEquals(non_smi_int32, o2.y);
+
+foo(true);
+assertUnoptimized(foo);
+assertEquals(47, cst);
+assertEquals(47, o1.x);
+assertEquals(52, o2.y);