[webkit-reviews] review granted: [Bug 200480] [JSC] Add "jump if (not) undefined or null" bytecode ops : [Attachment 375753] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Ross Kirsling
<ross.kirsling at sony.com>'s request for review:
Bug 200480: [JSC] Add "jump if (not) undefined or null" bytecode ops
https://bugs.webkit.org/show_bug.cgi?id=200480

Attachment 375753: Patch

https://bugs.webkit.org/attachment.cgi?id=375753&action=review




--- Comment #16 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 375753
  --> https://bugs.webkit.org/attachment.cgi?id=375753
Patch

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

r=me

>>> JSTests/stress/nullish-coalescing.js:25
>>> +for (let i = 0; i < 1e5; i++) {
>> 
>> I’m not sure whether this idiom of doing things in a loop to make sure the
gets JIT compiled is the standard way of making tests in JSTests/stress check
JIT correctness. But if it’s not 100% standard then I suggest adding a brief
comment explaining the technique and why 1e5 is a big enough number.
> 
> This is purely mimicry on my part, since I took this to be a ubiquitous idiom
in JSTests/stress -- in particular we have:
> 
>  55 files using `< 1e3`
> 221 files using `< 1e4`
> 101 files using `< 1e5`
>  89 files using `< 1e6`
>   2 files using `< 1e7`
> 
> 141 files using `< 1000`
> 578 files using `< 10000`
> 339 files using `< 100000`
>  61 files using `< 1000000`
>   6 files using `< 10000000`
> 
> Otherwise I have no commitment to 1e5 whatsoever.

Yeah this is a sensible number for JSC. We run each test in various JIT tier up
modes, so this will end up stressing all the tiers. However, if I have one nit,
I’d change the body of the loop to a function the loop calls. (That way if we
somehow break loop OSR entry we will still test the optimizing JITs.)