[for-of-performance] Check if next method is callable [v8/v8 : main]
1 view
Skip to first unread message
Rezvan Mahdavi Hezaveh (Gerrit)
Oct 20, 2025, 6:35:46 PM (6 days ago) Oct 20
to Leszek Swirski, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Rezvan Mahdavi Hezaveh added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 3 (Latest):
Rezvan Mahdavi Hezaveh . resolved
While I was working on the reported bug, I found that we never checked if the prototype has changed before going into fast path. I fixed both and added test case for them. PTAL, thanks.
Related details
Attention is currently required from:
- Leszek Swirski
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rezvan Mahdavi Hezaveh (Gerrit)
Oct 20, 2025, 8:28:46 PM (6 days ago) Oct 20
to AyeAye, Leszek Swirski, V8 LUCI CQ, verwaes...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Rezvan Mahdavi Hezaveh voted and added 1 comment![]( "Open in Gerrit")
Votes added by Rezvan Mahdavi Hezaveh
| Commit-Queue | +1 |
1 comment
File src/baseline/baseline-compiler.cc
Line 2493, Patchset 4 (Latest):
Register object = scratch_scope.AcquireScratch();
Register next = scratch_scope.AcquireScratch();
__ LoadRegister(object, RegisterOperand(0));
__ LoadRegister(next, RegisterOperand(1));
// Pass the output register slot as an argument, so that the builtin
// is responsible for writing into the slots.
Register out_reg_address = scratch_scope.AcquireScratch();
basm_.RegisterFrameAddress(RegisterOperand(2), out_reg_address);
CallBuiltin<Builtin::kForOfNextBaseline>(object, // object
next, // next
out_reg_address); // out_regRezvan Mahdavi Hezaveh . unresolved
This part is to fix the failure I was getting on my both CLs: https://ci.chromium.org/ui/p/v8/builders/try/v8_linux_rel/b8700456800073641505/overview
Related details
Attention is currently required from:
- Leszek Swirski
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. |
Rezvan Mahdavi Hezaveh (Gerrit)
Oct 23, 2025, 1:14:32 PM (3 days ago) Oct 23
to AyeAye, Leszek Swirski, V8 LUCI CQ, verwaes...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Rezvan Mahdavi Hezaveh added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Rezvan Mahdavi Hezaveh . resolved
friendly ping :)
Related details
Attention is currently required from:
- Leszek Swirski
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. |
Leszek Swirski (Gerrit)
Oct 24, 2025, 6:46:59 AM (2 days ago) Oct 24
to Rezvan Mahdavi Hezaveh, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com
Attention needed from Rezvan Mahdavi Hezaveh
Leszek Swirski voted and added 1 comment![]( "Open in Gerrit")
Votes added by Leszek Swirski
| Code-Review | +1 |
1 comment
File src/baseline/baseline-compiler.cc
Line 2493, Patchset 4 (Latest):
Register object = scratch_scope.AcquireScratch();
Register next = scratch_scope.AcquireScratch();
__ LoadRegister(object, RegisterOperand(0));
__ LoadRegister(next, RegisterOperand(1));
// Pass the output register slot as an argument, so that the builtin
// is responsible for writing into the slots.
Register out_reg_address = scratch_scope.AcquireScratch();
basm_.RegisterFrameAddress(RegisterOperand(2), out_reg_address);
CallBuiltin<Builtin::kForOfNextBaseline>(object, // object
next, // next
out_reg_address); // out_regRezvan Mahdavi Hezaveh . resolved
Leszek SwirskiThis part is to fix the failure I was getting on my both CLs: https://ci.chromium.org/ui/p/v8/builders/try/v8_linux_rel/b8700456800073641505/overview
I'm not 100% why this is broken in the previous version but if it fixes things then lgtm.
Related details
Attention is currently required from:
- Rezvan Mahdavi Hezaveh
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rezvan Mahdavi Hezaveh (Gerrit)
Oct 24, 2025, 4:33:17 PM (2 days ago) Oct 24
to Leszek Swirski, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com
Rezvan Mahdavi Hezaveh voted Commit-Queue+2![]( "Open in Gerrit")
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)
Oct 24, 2025, 5:35:49 PM (2 days ago) Oct 24
to Rezvan Mahdavi Hezaveh, Leszek Swirski, AyeAye, verwaes...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[for-of-performance] Check if next method is callable
This CL adds two checks: 1) if the iterator next method
is modified by the user, 2) if the next method is callable.
Bug: 452776165
Change-Id: I66199f7f4218b9febfe3f2bf9aa58ae06d160071
Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Rezvan Mahdavi Hezaveh <rez...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#103349}
Files:
- M src/baseline/baseline-compiler.cc
- M src/codegen/code-stub-assembler.cc
- A test/mjsunit/regress/regress-452776165.js
Change size: S
Delta: 3 files changed, 32 insertions(+), 4 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Leszek Swirski
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages