[maglev] Simplify the float conversion API [v8/v8 : main]
0 views
Skip to first unread message
Victor Gomes (Gerrit)
Aug 13, 2026, 7:45:27 AM (2 days ago) Aug 13
to Patrick Thier, v8-s...@luci-project-accounts.iam.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Patrick Thier
Victor Gomes voted and added 1 comment![]( "Open in Gerrit")
Votes added by Victor Gomes
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Patrick Thier
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Thier (Gerrit)
Aug 13, 2026, 10:45:56 AM (2 days ago) Aug 13
to Victor Gomes, v8-s...@luci-project-accounts.iam.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Victor Gomes
Patrick Thier voted and added 4 comments![]( "Open in Gerrit")
Votes added by Patrick Thier
| Code-Review | +1 |
4 comments
Patchset-level comments
Patrick Thier . resolved
LGTM
File src/maglev/maglev-reducer-inl.h
Line 1937, Patchset 10 (Latest):
DCHECK_NE(assumed_input_type, NodeType::kNumber);Patrick Thier . unresolved
I don't fully understand why this is a `DCHECK` here.
What's the invariant? Can you add a comment?
Line 1872, Patchset 10 (Latest):
// TODO(victorgomes): The type is impossible. We could generate an
// unconditional deopt here, but it's too invasive. So we just generate
// a check which will always deopt.Patrick Thier . unresolved
What's the difference between an unconditional deopt and a check that will always deopt?
Line 1867, Patchset 10 (Latest):
// What the caller lets the conversion assume, narrowed by what is
// already known about the value.Patrick Thier . unresolved
? I don't really understand this comment 😊
Related details
Attention is currently required from:
- Victor Gomes
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Victor Gomes (Gerrit)
Aug 14, 2026, 5:11:44 AM (yesterday) Aug 14
to Patrick Thier, v8-s...@luci-project-accounts.iam.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Victor Gomes added 4 comments![]( "Open in Gerrit")
Patchset-level comments
File src/maglev/maglev-reducer-inl.h
I don't fully understand why this is a `DCHECK` here.
What's the invariant? Can you add a comment?
Victor Gomes
Added a comment. Done.
Line 1872, Patchset 10:
// TODO(victorgomes): The type is impossible. We could generate an
// unconditional deopt here, but it's too invasive. So we just generate
// a check which will always deopt.Patrick Thier . resolved
What's the difference between an unconditional deopt and a check that will always deopt?
Victor Gomes
Unconditional would kill the branch. I'll do it in a follow up. I was just being careful here. This bid me before.
Line 1867, Patchset 10:
// What the caller lets the conversion assume, narrowed by what is
// already known about the value.Patrick Thier . resolved
? I don't really understand this comment 😊
Victor Gomes
node_info->type() is what we know so far (via KNA) about the node type.
assumed_input_type is what we can speculate.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
v8-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
Aug 14, 2026, 6:06:50 AM (yesterday) Aug 14
to Victor Gomes, Patrick Thier, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
v8-s...@luci-project-accounts.iam.gserviceaccount.com submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
10 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: 10, Deletions: 5.
The diff is too large to show. Please review the diff.
```
Change information
Commit message:
[maglev] Simplify the float conversion API
Bug: 545843233
Change-Id: Ib38f9694189f35f3543c355cd09077602cc5332a
Reviewed-by: Patrick Thier <pth...@chromium.org>
Commit-Queue: Victor Gomes <victo...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#109252}
Files:
- 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-reducer-inl.h
- M src/maglev/maglev-reducer.h
- A test/mjsunit/maglev/float64-conversions.js
Change size: L
Delta: 6 files changed, 600 insertions(+), 300 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Patrick Thier
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages