[turbofan] Insert CheckString before CheckEqualsInternalizedString [v8/v8 : main]
0 views
Skip to first unread message
Darius Mercadier (Gerrit)
4:38 AM (7 hours ago) 4:38 AM
to Nico Hartmann, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Nico Hartmann
Darius Mercadier voted and added 1 comment![]( "Open in Gerrit")
Votes added by Darius Mercadier
| Auto-Submit | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Nico Hartmann
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nico Hartmann (Gerrit)
5:25 AM (6 hours ago) 5:25 AM
to Darius Mercadier, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Darius Mercadier
Nico Hartmann voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nico Hartmann
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Darius Mercadier
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Darius Mercadier (Gerrit)
5:28 AM (6 hours ago) 5:28 AM
to Nico Hartmann, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
Darius Mercadier voted and added 2 comments![]( "Open in Gerrit")
Votes added by Darius Mercadier
| Commit-Queue | +2 |
2 comments
Patchset-level comments
Nico Hartmann . resolved
Why didn't we notice this yet?
Darius Mercadier
I think that it's because 1) when the input isn't a string, CheckEqualsInternalizedString will almost always deopt because it will treat it as a string and it's very unlikely that reading random fields from a random objects produces the expected string, and 2) we were still inserting a CheckString (as far as I can see) but after the CheckEqualsInternalizedString, so even if by chance we had a random object that passed CheckEqualsInternalizedString, then we would have deopted afterwards on the CheckString.
My fix is mainly to make the graph more sound, and to avoid loading string fields from non-string objects.
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
5:29 AM (6 hours ago) 5:29 AM
to Darius Mercadier, Nico Hartmann, dmercadi...@chromium.org, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[turbofan] Insert CheckString before CheckEqualsInternalizedString
... because CheckEqualsInternalizedString assumes that its input is
a String.
Fixed: 475299908
Change-Id: I489c6c4d9c73375682432ed78cdfdfcb065fc6e6
Commit-Queue: Darius Mercadier <dmerc...@chromium.org>
Auto-Submit: Darius Mercadier <dmerc...@chromium.org>
Reviewed-by: Nico Hartmann <nicoha...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#104660}
Files:
- M src/compiler/js-native-context-specialization.cc
- A test/mjsunit/turboshaft/regress-475299908.js
Change size: S
Delta: 2 files changed, 26 insertions(+), 3 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Nico Hartmann
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages