[turbolev] Enable load elimination for non-constant loads [v8/v8 : main]

0 views
Skip to first unread message

Victor Gomes (Gerrit)

unread,
Oct 22, 2025, 11:58:49 AM (24 hours ago) Oct 22
to Darius Mercadier, V8 LUCI CQ, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Darius Mercadier

Victor Gomes voted and added 1 comment

Votes added by Victor Gomes

Commit-Queue+1

1 comment

Patchset-level comments
File-level comment, Patchset 2 (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: Ic483cc348514b9427f2b20d6fff25d7945b73fb2
Gerrit-Change-Number: 7071735
Gerrit-PatchSet: 2
Gerrit-Owner: Victor Gomes <victo...@chromium.org>
Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
Gerrit-Reviewer: Victor Gomes <victo...@chromium.org>
Gerrit-Attention: Darius Mercadier <dmerc...@chromium.org>
Gerrit-Comment-Date: Wed, 22 Oct 2025 15:58:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Darius Mercadier (Gerrit)

unread,
3:01 AM (8 hours ago) 3:01 AM
to Victor Gomes, V8 LUCI CQ, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Victor Gomes

Darius Mercadier voted and added 2 comments

Votes added by Darius Mercadier

Code-Review+1

2 comments

Patchset-level comments
Darius Mercadier . resolved

LGTM % comment

File src/maglev/maglev-graph-builder.cc
Line 10420, Patchset 2 (Latest): StoreTaggedMode::kDefault, broker()->length_string()));
Darius Mercadier . unresolved

I'm not a big fan of this. Have an additional kLength property type would be cleaner imo.

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: Ic483cc348514b9427f2b20d6fff25d7945b73fb2
Gerrit-Change-Number: 7071735
Gerrit-PatchSet: 2
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 Oct 2025 07:00:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Victor Gomes (Gerrit)

unread,
3:03 AM (8 hours ago) 3:03 AM
to Darius Mercadier, V8 LUCI CQ, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org

Victor Gomes voted and added 2 comments

Votes added by Victor Gomes

Commit-Queue+2

2 comments

Patchset-level comments
Victor Gomes . resolved

Thanks!

File src/maglev/maglev-graph-builder.cc
Line 10420, Patchset 2 (Latest): StoreTaggedMode::kDefault, broker()->length_string()));
Darius Mercadier . resolved

I'm not a big fan of this. Have an additional kLength property type would be cleaner imo.

Victor Gomes

Right, but for correctness for now I'll put like that. Since we are keying the cache with the string length from a different part of the code.

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: Ic483cc348514b9427f2b20d6fff25d7945b73fb2
    Gerrit-Change-Number: 7071735
    Gerrit-PatchSet: 2
    Gerrit-Owner: Victor Gomes <victo...@chromium.org>
    Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
    Gerrit-Reviewer: Victor Gomes <victo...@chromium.org>
    Gerrit-Comment-Date: Thu, 23 Oct 2025 07:03:37 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Darius Mercadier <dmerc...@chromium.org>
    satisfied_requirement
    open
    diffy

    V8 LUCI CQ (Gerrit)

    unread,
    3:05 AM (8 hours ago) 3:05 AM
    to Victor Gomes, Darius Mercadier, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org

    V8 LUCI CQ submitted the change

    Change information

    Commit message:
    [turbolev] Enable load elimination for non-constant loads

    Follow-up of: https://crrev.com/c/7071831
    Bug: 424157317
    Change-Id: Ic483cc348514b9427f2b20d6fff25d7945b73fb2
    Reviewed-by: Darius Mercadier <dmerc...@chromium.org>
    Commit-Queue: Victor Gomes <victo...@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#103303}
    Files:
    • M src/maglev/maglev-graph-builder.cc
    • M src/maglev/maglev-graph-optimizer.cc
    • M src/maglev/maglev-ir.cc
    • M src/maglev/maglev-known-node-aspects.h
    Change size: S
    Delta: 4 files changed, 26 insertions(+), 8 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: Ic483cc348514b9427f2b20d6fff25d7945b73fb2
    Gerrit-Change-Number: 7071735
    Gerrit-PatchSet: 3
    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