[maglev] Fix conversion node leak from FixedDoubleArray vobject load [v8/v8 : main]
0 views
Skip to first unread message
Victor Gomes (Gerrit)
9:52 AM (10 hours ago) 9:52 AM
to Jakob Linke, v8-s...@luci-project-accounts.iam.gserviceaccount.com, android-bu...@system.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Jakob Linke
Victor Gomes voted and added 1 comment![]( "Open in Gerrit")
Votes added by Victor Gomes
| Auto-Submit | +1 |
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Jakob Linke
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Linke (Gerrit)
9:58 AM (10 hours ago) 9:58 AM
to Victor Gomes, v8-s...@luci-project-accounts.iam.gserviceaccount.com, android-bu...@system.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Victor Gomes
Jakob Linke voted and added 1 comment![]( "Open in Gerrit")
Votes added by Jakob Linke
| Code-Review | +1 |
1 comment
File src/maglev/maglev-graph-builder.cc
Line 4901, Patchset 4 (Latest):
return vobject->get(FixedDoubleArray::OffsetOfElementAt(index))Jakob Linke . unresolved
Should we move Unwrap into `get`? Just a thought for followups
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)
10:00 AM (10 hours ago) 10:00 AM
to Jakob Linke, v8-s...@luci-project-accounts.iam.gserviceaccount.com, android-bu...@system.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Victor Gomes voted and added 2 comments![]( "Open in Gerrit")
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 4901, Patchset 4 (Latest):
return vobject->get(FixedDoubleArray::OffsetOfElementAt(index))Jakob Linke . resolved
Should we move Unwrap into `get`? Just a thought for followups
Victor Gomes
Maybe... i was in a fence here. I like the fact that we know where conversions could be coming from, but maybe that's not that useful.
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)
11:02 AM (9 hours ago) 11:02 AM
to Victor Gomes, Jakob Linke, android-bu...@system.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![]( "Open in Gerrit")
Change information
Commit message:
[maglev] Fix conversion node leak from FixedDoubleArray vobject load
When --maglev-object-tracking is enabled, BuildLoadFixedDoubleArrayElement
returns the value stored in a virtual FixedDoubleArray directly. For
arrays initialized via the inlined Array(...) constructor with elements
of PACKED_DOUBLE_ELEMENTS kind, the stored values can be conversion nodes
(e.g. ChangeInt32ToFloat64), which is permitted for initializing stores.
However, conversion nodes must not leak into the interpreter frame state
(set/set_accumulator DCHECKs on !value->is_conversion()), since
conversions belong in NodeInfo as alternative representations.
Unwrap the conversion when reading from the virtual object, returning
the underlying input value. The Float64 alternative remains available in
NodeInfo for callers that need it.
Bug: 500177421
Change-Id: I319acf53f5910ef82087ae04c37aa6b4621e5adf
Auto-Submit: Victor Gomes <victo...@chromium.org>
Reviewed-by: Jakob Linke <jgr...@chromium.org>
Commit-Queue: Victor Gomes <victo...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#106894}
Files:
- M src/maglev/maglev-graph-builder.cc
- A test/mjsunit/maglev/regress-500177421.js
Change size: S
Delta: 2 files changed, 27 insertions(+), 1 deletion(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Jakob Linke
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages