Bug 205124 - Add test for osr exiting with interpreter on the stack
Summary: Add test for osr exiting with interpreter on the stack
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-11 10:32 PST by Keith Miller
Modified: 2019-12-11 12:15 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.51 KB, patch)
2019-12-11 10:32 PST, Keith Miller
no flags Details | Formatted Diff | Diff
Patch for landing (1.51 KB, patch)
2019-12-11 11:31 PST, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2019-12-11 10:32:08 PST
Add test for osr exiting with interpreter on the stack
Comment 1 Keith Miller 2019-12-11 10:32:59 PST
Created attachment 385406 [details]
Patch
Comment 2 Mark Lam 2019-12-11 10:45:10 PST
Comment on attachment 385406 [details]
Patch

rs=me.  Make sure EWS bots are happy.
Comment 3 Mark Lam 2019-12-11 11:08:17 PST
Comment on attachment 385406 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385406&action=review

> JSTests/stress/osr-exit-with-interpreter-on-stack.js:1
> +//@ requiredOptions('--thresholdForJITAfterWarmUp=10', '--useConcurrentJIT=0')

/requiredOptions/requireOptions/

nit: use '--useConcurrentJIT=false' instead.  I've been told in the past that though the bool option is able to take an int, using it in official code is a no-no.  So, let's just avoid that.
Comment 4 Mark Lam 2019-12-11 11:23:29 PST
<rdar://problem/55917224>
Comment 5 Keith Miller 2019-12-11 11:31:21 PST
Created attachment 385416 [details]
Patch for landing
Comment 6 Keith Miller 2019-12-11 11:32:24 PST
Comment on attachment 385406 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385406&action=review

>> JSTests/stress/osr-exit-with-interpreter-on-stack.js:1
>> +//@ requiredOptions('--thresholdForJITAfterWarmUp=10', '--useConcurrentJIT=0')
> 
> /requiredOptions/requireOptions/
> 
> nit: use '--useConcurrentJIT=false' instead.  I've been told in the past that though the bool option is able to take an int, using it in official code is a no-no.  So, let's just avoid that.

Whoops, fixed.

Ehh, we use 0/1 more than true/false currently so I'm gonna stick with 0.
Comment 7 WebKit Commit Bot 2019-12-11 12:15:33 PST
Comment on attachment 385416 [details]
Patch for landing

Clearing flags on attachment: 385416

Committed r253384: <https://trac.webkit.org/changeset/253384>
Comment 8 WebKit Commit Bot 2019-12-11 12:15:34 PST
All reviewed patches have been landed.  Closing bug.