[maglev] Inline Object.is [v8/v8 : main]

0 views
Skip to first unread message

Victor Gomes (Gerrit)

unread,
Jul 23, 2026, 9:19:17 AM (2 days ago) Jul 23
to Darius Mercadier, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-mip...@googlegroups.com, v8-ppc...@googlegroups.com, v8-re...@googlegroups.com, v8-risc...@chromium.org, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Darius Mercadier

Victor Gomes added 1 comment

Patchset-level comments
File-level comment, Patchset 3 (Latest):
Victor Gomes . resolved

PTAL!

Open in Gerrit

Related details

Attention is currently required from:
  • Darius Mercadier
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I1b6b98040db037841f2a3107cadb878c868c4b78
Gerrit-Change-Number: 8137280
Gerrit-PatchSet: 3
Gerrit-Owner: Victor Gomes <victo...@chromium.org>
Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
Gerrit-Attention: Darius Mercadier <dmerc...@chromium.org>
Gerrit-Comment-Date: Thu, 23 Jul 2026 13:19:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Darius Mercadier (Gerrit)

unread,
Jul 23, 2026, 9:55:31 AM (2 days ago) Jul 23
to Victor Gomes, v8-s...@luci-project-accounts.iam.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-mip...@googlegroups.com, v8-ppc...@googlegroups.com, v8-re...@googlegroups.com, v8-risc...@chromium.org, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Victor Gomes

Darius Mercadier voted and added 11 comments

Votes added by Darius Mercadier

Code-Review+1

11 comments

Patchset-level comments
Darius Mercadier . resolved

lgtm

File src/maglev/maglev-graph-optimizer.cc
Line 3193, Patchset 3 (Latest): auto right = reducer_.TryGetFloat64OrHoleyFloat64Constant(
Darius Mercadier . unresolved

newline

Line 3197, Patchset 3 (Latest): return ReplaceWith(reducer_.GetBooleanConstant(
Darius Mercadier . unresolved

newline

File src/maglev/maglev-reducer-inl.h
Line 2780, Patchset 3 (Latest): return BuildCallBuiltinWithTaggedInputs<Builtin::kSameValue>({lhs, rhs});
Darius Mercadier . unresolved

Would be nice to have a high-level node for this so that escape analysis can constant-fold when one side is an elided object but the other side isn't (this should always produce false, right?).

Line 2775, Patchset 3 (Latest): CheckType(rhs, NodeType::kString)) {
Darius Mercadier . unresolved

same

Line 2774, Patchset 3 (Latest): } else if (CheckType(lhs, NodeType::kString) &&
Darius Mercadier . unresolved

same

Line 2772, Patchset 3 (Latest): CheckType(rhs, kReferenceEqual)) {
Darius Mercadier . unresolved

same

Line 2771, Patchset 3 (Latest): } else if (CheckType(lhs, kReferenceEqual) ||
Darius Mercadier . unresolved

same

Line 2769, Patchset 3 (Latest): if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {
Darius Mercadier . unresolved

`NodeTypeIs(lhs_type`

Line 2769, Patchset 3 (Latest): if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {
Darius Mercadier . unresolved

`NodeTypeIs(rhs_type`

Line 2719, Patchset 3 (Latest): ValueNode* int32_lhs = TryGetInt32(lhs);
Darius Mercadier . unresolved

DCHECK that the type of the inputs is Number here maybe.

Open in Gerrit

Related details

Attention is currently required from:
  • Victor Gomes
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I1b6b98040db037841f2a3107cadb878c868c4b78
Gerrit-Change-Number: 8137280
Gerrit-PatchSet: 3
Gerrit-Owner: Victor Gomes <victo...@chromium.org>
Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
Gerrit-Reviewer: Victor Gomes <victo...@chromium.org>
Gerrit-Attention: Victor Gomes <victo...@chromium.org>
Gerrit-Comment-Date: Thu, 23 Jul 2026 13:55:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Victor Gomes (Gerrit)

unread,
Jul 23, 2026, 11:44:11 AM (2 days ago) Jul 23
to Darius Mercadier, v8-s...@luci-project-accounts.iam.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-mip...@googlegroups.com, v8-ppc...@googlegroups.com, v8-re...@googlegroups.com, v8-risc...@chromium.org, verwaes...@chromium.org, victorgo...@chromium.org

Victor Gomes voted and added 21 comments

Votes added by Victor Gomes

Commit-Queue+2

21 comments

Patchset-level comments
File-level comment, Patchset 4:
Victor Gomes . resolved

Done

File-level comment, Patchset 4:
Victor Gomes . resolved

Done

File-level comment, Patchset 4:
Victor Gomes . resolved

Done

File-level comment, Patchset 4:
Victor Gomes . resolved

Done (using CheckType, which is side-effect free, so it's DCHECK-safe).

File-level comment, Patchset 4:
Victor Gomes . resolved

Done

File-level comment, Patchset 4:
Victor Gomes . resolved

Added a TODO. And yes: a fresh (elided) allocation is only SameValue to that very object, so pairing it with any distinct object or non-object always yields false — a high-level node would let escape analysis fold exactly that case. Left it as a follow-up to keep this CL focused.

File-level comment, Patchset 4:
Victor Gomes . resolved

Done

File-level comment, Patchset 4:
Victor Gomes . resolved

Done

File-level comment, Patchset 4:
Victor Gomes . resolved

Done

File-level comment, Patchset 4:
Victor Gomes . resolved

Done

File-level comment, Patchset 5 (Latest):
Victor Gomes . resolved

Thanks!

File src/maglev/maglev-graph-optimizer.cc
Line 3193, Patchset 3: auto right = reducer_.TryGetFloat64OrHoleyFloat64Constant(
Darius Mercadier . resolved

newline

Victor Gomes

Done

Line 3197, Patchset 3: return ReplaceWith(reducer_.GetBooleanConstant(
Darius Mercadier . resolved

newline

Victor Gomes

Done

File src/maglev/maglev-reducer-inl.h
Line 2780, Patchset 3: return BuildCallBuiltinWithTaggedInputs<Builtin::kSameValue>({lhs, rhs});
Darius Mercadier . resolved

Would be nice to have a high-level node for this so that escape analysis can constant-fold when one side is an elided object but the other side isn't (this should always produce false, right?).

Victor Gomes

I mean you could just inspect CallBuiltin<kSameValue>, no?
Similar argument might be true for other builtins.

Line 2775, Patchset 3: CheckType(rhs, NodeType::kString)) {
Darius Mercadier . resolved

same

Victor Gomes

Done

Line 2774, Patchset 3: } else if (CheckType(lhs, NodeType::kString) &&
Darius Mercadier . resolved

same

Victor Gomes

Done

Line 2772, Patchset 3: CheckType(rhs, kReferenceEqual)) {
Darius Mercadier . resolved

same

Victor Gomes

Done

Line 2771, Patchset 3: } else if (CheckType(lhs, kReferenceEqual) ||
Darius Mercadier . resolved

same

Victor Gomes

Done

Line 2769, Patchset 3: if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {
Darius Mercadier . resolved

`NodeTypeIs(lhs_type`

Victor Gomes

Done

Line 2769, Patchset 3: if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {
Darius Mercadier . resolved

`NodeTypeIs(rhs_type`

Victor Gomes

Done

Line 2719, Patchset 3: ValueNode* int32_lhs = TryGetInt32(lhs);
Darius Mercadier . resolved

DCHECK that the type of the inputs is Number here maybe.

Victor Gomes

Done

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: I1b6b98040db037841f2a3107cadb878c868c4b78
    Gerrit-Change-Number: 8137280
    Gerrit-PatchSet: 5
    Gerrit-Comment-Date: Thu, 23 Jul 2026 15:44:06 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Darius Mercadier <dmerc...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    chromeperf@appspot.gserviceaccount.com (Gerrit)

    unread,
    Jul 24, 2026, 3:47:36 AM (yesterday) Jul 24
    to Victor Gomes, Darius Mercadier, v8-s...@luci-project-accounts.iam.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-mip...@googlegroups.com, v8-ppc...@googlegroups.com, v8-re...@googlegroups.com, v8-risc...@chromium.org, verwaes...@chromium.org, victorgo...@chromium.org
    Attention needed from Victor Gomes

    Message from chrom...@appspot.gserviceaccount.com

    📍 Job mac-m4-mini-perf/jetstream-main.crossbench complete.

    See results at: https://pinpoint-dot-chromeperf.appspot.com/job/16b34596290000

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Victor Gomes
    Submit Requirements:
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: v8/v8
      Gerrit-Branch: main
      Gerrit-Change-Id: I1b6b98040db037841f2a3107cadb878c868c4b78
      Gerrit-Change-Number: 8137280
      Gerrit-PatchSet: 6
      Gerrit-Attention: Victor Gomes <victo...@chromium.org>
      Gerrit-Comment-Date: Fri, 24 Jul 2026 07:47:31 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: No
      satisfied_requirement
      open
      diffy

      Victor Gomes (Gerrit)

      unread,
      Jul 24, 2026, 3:50:40 AM (yesterday) Jul 24
      to Darius Mercadier, v8-s...@luci-project-accounts.iam.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-mip...@googlegroups.com, v8-ppc...@googlegroups.com, v8-re...@googlegroups.com, v8-risc...@chromium.org, verwaes...@chromium.org, victorgo...@chromium.org

      Victor Gomes voted Commit-Queue+2

      Commit-Queue+2
      Open in Gerrit

      Related details

      Attention set is empty
      Submit Requirements:
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: v8/v8
      Gerrit-Branch: main
      Gerrit-Change-Id: I1b6b98040db037841f2a3107cadb878c868c4b78
      Gerrit-Change-Number: 8137280
      Gerrit-PatchSet: 6
      Gerrit-Owner: Victor Gomes <victo...@chromium.org>
      Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
      Gerrit-Reviewer: Victor Gomes <victo...@chromium.org>
      Gerrit-Comment-Date: Fri, 24 Jul 2026 07:50:36 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      open
      diffy

      v8-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)

      unread,
      Jul 24, 2026, 3:58:06 AM (yesterday) Jul 24
      to Victor Gomes, Darius Mercadier, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-mip...@googlegroups.com, v8-ppc...@googlegroups.com, v8-re...@googlegroups.com, v8-risc...@chromium.org, verwaes...@chromium.org, victorgo...@chromium.org

      v8-s...@luci-project-accounts.iam.gserviceaccount.com submitted the change with unreviewed changes

      Unreviewed changes

      3 is the latest approved patch-set.
      The change was submitted with unreviewed changes in the following files:

      ```
      The name of the file: src/maglev/maglev-reducer-inl.h
      Insertions: 8, Deletions: 5.

      @@ -2716,6 +2716,8 @@
      template <typename BaseT>
      ReduceResult MaglevReducer<BaseT>::BuildNumberSameValue(ValueNode* lhs,
      ValueNode* rhs) {
      + DCHECK(CheckType(lhs, NodeType::kNumber));
      + DCHECK(CheckType(rhs, NodeType::kNumber));
      ValueNode* int32_lhs = TryGetInt32(lhs);
      ValueNode* int32_rhs = TryGetInt32(rhs);
      if (int32_lhs != nullptr && int32_rhs != nullptr) {
      @@ -2766,13 +2768,14 @@
      const NodeType kReferenceEqual = UnionType(
      NodeType::kJSReceiver, UnionType(NodeType::kSymbol, NodeType::kOddball));

      - if (CheckType(lhs, NodeType::kNumber) && CheckType(rhs, NodeType::kNumber)) {
      + if (NodeTypeIs(lhs_type, NodeType::kNumber) &&
      + NodeTypeIs(rhs_type, NodeType::kNumber)) {
      return BuildNumberSameValue(lhs, rhs);
      - } else if (CheckType(lhs, kReferenceEqual) ||
      - CheckType(rhs, kReferenceEqual)) {
      + } else if (NodeTypeIs(lhs_type, kReferenceEqual) ||
      + NodeTypeIs(rhs_type, kReferenceEqual)) {
      return BuildTaggedEqual(lhs, rhs);
      - } else if (CheckType(lhs, NodeType::kString) &&
      - CheckType(rhs, NodeType::kString)) {
      + } else if (NodeTypeIs(lhs_type, NodeType::kString) &&
      + NodeTypeIs(rhs_type, NodeType::kString)) {
      return AddNewNode<StringEqual>({lhs, rhs},
      StringEqualInputMode::kOnlyStrings);
      }
      @@ -4703,11 +4706,11 @@
      template <typename BaseT>
      ReduceResult MaglevReducer<BaseT>::BuildLoadJSDataViewByteLength(
      ValueNode* js_data_view) {
      + bool is_const = !v8_flags.track_array_buffer_views;
      // Note: We can't use broker()->byte_length_string() here, because it could
      // conflict with redefinitions of the ArrayBufferView byteLength property.
      - if (ValueNode* byte_length =
      - known_node_aspects().TryFindLoadedConstantProperty(
      - js_data_view, PropertyKey::ArrayBufferViewByteLength())) {
      + if (ValueNode* byte_length = known_node_aspects().TryFindLoadedProperty(
      + js_data_view, PropertyKey::ArrayBufferViewByteLength(), is_const)) {
      return byte_length;
      }

      @@ -4715,16 +4718,16 @@
      GET_VALUE_OR_ABORT(result,
      AddNewNode<LoadDataViewByteLength>({js_data_view}));
      RecordKnownProperty(js_data_view, PropertyKey::ArrayBufferViewByteLength(),
      - result, true, compiler::AccessMode::kLoad);
      + result, is_const, compiler::AccessMode::kLoad);
      return result;
      }

      template <typename BaseT>
      ReduceResult MaglevReducer<BaseT>::BuildLoadJSDataViewDataPointer(
      ValueNode* js_data_view) {
      - if (ValueNode* backing_store =
      - known_node_aspects().TryFindLoadedConstantProperty(
      - js_data_view, PropertyKey::ArrayBufferViewDataPointer())) {
      + bool is_const = !v8_flags.track_array_buffer_views;
      + if (ValueNode* backing_store = known_node_aspects().TryFindLoadedProperty(
      + js_data_view, PropertyKey::ArrayBufferViewDataPointer(), is_const)) {
      return backing_store;
      }

      @@ -4732,7 +4735,7 @@
      GET_VALUE_OR_ABORT(result,
      AddNewNode<LoadDataViewDataPointer>({js_data_view}));
      RecordKnownProperty(js_data_view, PropertyKey::ArrayBufferViewDataPointer(),
      - result, true, compiler::AccessMode::kLoad);
      + result, is_const, compiler::AccessMode::kLoad);
      return result;
      }

      ```
      ```
      The name of the file: test/mjsunit/maglev/object-is.js
      Insertions: 22, Deletions: 13.

      @@ -72,11 +72,12 @@
      (function() {
      function smiZero(o) { return Object.is(+o, 0); }
      function int32Zero(a, b) { return Object.is(+a, b | 0); }
      - function constants() {
      - return [Object.is(-0, 0), Object.is(0, -0), Object.is(-0, 0.0),
      - Object.is(-0, Math.max(-0.0, 0.0)), Object.is(-0, Math.min(0.0, 0.0)),
      - Object.is(-0, Math.min(-0.0, 0.0))];
      - }
      + // Both operands constant: a zero constant must not fold via numeric equality.
      + // Math.max(-0, 0) is +0, Math.min(-0, 0) is -0.
      + function negZeroVsZero() { return Object.is(-0, 0); }
      + function zeroVsNegZero() { return Object.is(0, -0); }
      + function negZeroVsMax() { return Object.is(-0, Math.max(-0.0, 0.0)); }
      + function negZeroVsMin() { return Object.is(-0, Math.min(-0.0, 0.0)); }
      testOptimized(() => {
      assertFalse(smiZero(-0));
      assertTrue(smiZero(0));
      @@ -84,8 +85,11 @@
      assertFalse(int32Zero(-0, 0));
      assertTrue(int32Zero(0, 0));
      assertFalse(int32Zero(NaN, 0));
      - assertEquals([false, false, false, false, false, true], constants());
      - }, smiZero, int32Zero, constants);
      + assertFalse(negZeroVsZero());
      + assertFalse(zeroVsNegZero());
      + assertFalse(negZeroVsMax());
      + assertTrue(negZeroVsMin());
      + }, smiZero, int32Zero, negZeroVsZero, zeroVsNegZero, negZeroVsMax, negZeroVsMin);
      })();

      // The full Float64 same-value comparison.
      @@ -130,13 +134,18 @@

      // Constant folding of two number constants.
      (function() {
      - function foo() {
      - return [Object.is(NaN, NaN), Object.is(0, -0), Object.is(-0, -0),
      - Object.is(1, 1.0), Object.is(1, 2)];
      - }
      + function nanNan() { return Object.is(NaN, NaN); }
      + function zeroNegZero() { return Object.is(0, -0); }
      + function negZeroNegZero() { return Object.is(-0, -0); }
      + function oneOne() { return Object.is(1, 1.0); }
      + function oneTwo() { return Object.is(1, 2); }
      testOptimized(() => {
      - assertEquals([true, false, true, true, false], foo());
      - }, foo);
      + assertTrue(nanNan());
      + assertFalse(zeroNegZero());
      + assertTrue(negZeroNegZero());
      + assertTrue(oneOne());
      + assertFalse(oneTwo());
      + }, nanNan, zeroNegZero, negZeroNegZero, oneOne, oneTwo);
      })();

      // JSReceivers, symbols and oddballs compare by reference.
      ```
      ```
      The name of the file: src/maglev/maglev-graph-optimizer.cc
      Insertions: 2, Deletions: 0.

      @@ -3190,10 +3190,12 @@
      UseRepresentation::kFloat64, node->input_node(0),
      TaggedToFloat64ConversionType::kOnlyNumber);
      if (!left) return ProcessResult::kContinue;
      +
      auto right = reducer_.TryGetFloat64OrHoleyFloat64Constant(
      UseRepresentation::kFloat64, node->input_node(1),
      TaggedToFloat64ConversionType::kOnlyNumber);
      if (!right) return ProcessResult::kContinue;
      +
      return ReplaceWith(reducer_.GetBooleanConstant(
      Object::SameNumberValue(left->get_scalar(), right->get_scalar())));
      }
      ```

      Change information

      Commit message:
      [maglev] Inline Object.is

      Adds a Float64SameValue node for the general two-numbers case; the
      remaining shapes reuse existing comparison nodes.
      Bug: 402464049
      Change-Id: I1b6b98040db037841f2a3107cadb878c868c4b78
      Reviewed-by: Darius Mercadier <dmerc...@chromium.org>
      Commit-Queue: Victor Gomes <victo...@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#108859}
      Files:
      • M src/compiler/turboshaft/turbolev-graph-builder.cc
      • M src/maglev/arm/maglev-assembler-arm-inl.h
      • M src/maglev/arm64/maglev-assembler-arm64-inl.h
      • M src/maglev/loong64/maglev-assembler-loong64-inl.h
      • M src/maglev/maglev-assembler.h
      • M src/maglev/maglev-graph-builder.cc
      • M src/maglev/maglev-graph-builder.h
      • M src/maglev/maglev-graph-optimizer.cc
      • M src/maglev/maglev-ir.cc
      • M src/maglev/maglev-ir.h
      • M src/maglev/maglev-reducer-inl.h
      • M src/maglev/maglev-reducer.h
      • M src/maglev/ppc/maglev-assembler-ppc-inl.h
      • M src/maglev/riscv/maglev-assembler-riscv-inl.h
      • M src/maglev/s390/maglev-assembler-s390-inl.h
      • M src/maglev/x64/maglev-assembler-x64-inl.h
      • A test/mjsunit/maglev/object-is.js
      Change size: L
      Delta: 17 files changed, 512 insertions(+), 30 deletions(-)
      Branch: refs/heads/main
      Submit Requirements:
      • requirement satisfiedCode-Review: +1 by Darius Mercadier
      Open in Gerrit
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: merged
      Gerrit-Project: v8/v8
      Gerrit-Branch: main
      Gerrit-Change-Id: I1b6b98040db037841f2a3107cadb878c868c4b78
      Gerrit-Change-Number: 8137280
      Gerrit-PatchSet: 7
      Gerrit-Owner: Victor Gomes <victo...@chromium.org>
      Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
      Gerrit-Reviewer: Victor Gomes <victo...@chromium.org>
      open
      diffy
      satisfied_requirement
      Reply all
      Reply to author
      Forward
      0 new messages