[for-of-performance] Add ForOfNext builting to MayDeopt list [v8/v8 : main]
Rezvan Mahdavi Hezaveh (Gerrit)
Rezvan Mahdavi Hezaveh added 1 comment![]( "Open in Gerrit")
I know you removed the builtin from MayDeopt list in https://chromium-review.googlesource.com/c/v8/v8/+/6964615, but that caused 88 test failures. I think we need to put it back. PTAL, thanks!
P.S: By adding the builtin to this list all 88 test failures got resolved and no new test failure happened.
Related details
- Leszek Swirski
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Leszek Swirski added 1 comment![]( "Open in Gerrit")
I know you removed the builtin from MayDeopt list in https://chromium-review.googlesource.com/c/v8/v8/+/6964615, but that caused 88 test failures. I think we need to put it back. PTAL, thanks!
P.S: By adding the builtin to this list all 88 test failures got resolved and no new test failure happened.
I think there might be a missing test in that case, where someone sets a breakpoint in a function with for-of while paused inside the next() method of the iterator. Let me take a look at the failures, it might be that our DCHECK here is too eager.
Related details
- Rezvan Mahdavi Hezaveh
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)
Leszek Swirski added 1 comment![]( "Open in Gerrit")
Leszek SwirskiI know you removed the builtin from MayDeopt list in https://chromium-review.googlesource.com/c/v8/v8/+/6964615, but that caused 88 test failures. I think we need to put it back. PTAL, thanks!
P.S: By adding the builtin to this list all 88 test failures got resolved and no new test failure happened.
I think there might be a missing test in that case, where someone sets a breakpoint in a function with for-of while paused inside the next() method of the iterator. Let me take a look at the failures, it might be that our DCHECK here is too eager.
This is actually quite complicated, I'll need to think about it a bit -- the problem is that we _can_ deopt the ForOfNext bytecode (as mentioned, because of a breakpoint), and it will currently have the wrong value in the done/value registers after that deopt, which is what the failing DCHECK is guarding against.
Related details
- Rezvan Mahdavi Hezaveh
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)
Rezvan Mahdavi Hezaveh abandoned this change![]( "Open in Gerrit")
Related details
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)
Rezvan Mahdavi Hezaveh added 1 comment![]( "Open in Gerrit")
Leszek SwirskiI know you removed the builtin from MayDeopt list in https://chromium-review.googlesource.com/c/v8/v8/+/6964615, but that caused 88 test failures. I think we need to put it back. PTAL, thanks!
P.S: By adding the builtin to this list all 88 test failures got resolved and no new test failure happened.
Leszek SwirskiI think there might be a missing test in that case, where someone sets a breakpoint in a function with for-of while paused inside the next() method of the iterator. Let me take a look at the failures, it might be that our DCHECK here is too eager.
This is actually quite complicated, I'll need to think about it a bit -- the problem is that we _can_ deopt the ForOfNext bytecode (as mentioned, because of a breakpoint), and it will currently have the wrong value in the done/value registers after that deopt, which is what the failing DCHECK is guarding against.
Got it! Thanks for digging into this and preparing the other cLs. I am going to abandon this one then.
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |