Bug 206571

Summary: [JSC] Add CheckArrayOrEmpty to handle the case when hoisting CheckArray for places where input can be empty
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch saam: review+

Description Yusuke Suzuki 2020-01-21 23:12:21 PST
[JSC] Add CheckArrayOrEmpty to handle the case when hoisting CheckArray for places where input can be empty
Comment 1 Yusuke Suzuki 2020-01-21 23:18:38 PST
Created attachment 388398 [details]
Patch
Comment 2 Yusuke Suzuki 2020-01-21 23:19:10 PST
<rdar://problem/58757016>
Comment 3 Saam Barati 2020-01-22 08:33:03 PST
Comment on attachment 388398 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:287
> +                // check just remains, and it works as CheckArrayOrEmpty without ArrayMode checking.

Seems like we should also have an assert here like
speculatedTypeForUseKind(node->child1().useKind()) & SpecEmpty

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:832
> +        // We can purge Empty check completely in this case of CheckArrayOrEmpty since CellUse only accepts SpecCell | SpecEmpty.

Ditto
Comment 4 Yusuke Suzuki 2020-01-22 12:33:06 PST
Comment on attachment 388398 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:287
>> +                // check just remains, and it works as CheckArrayOrEmpty without ArrayMode checking.
> 
> Seems like we should also have an assert here like
> speculatedTypeForUseKind(node->child1().useKind()) & SpecEmpty

Fixed.

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:832
>> +        // We can purge Empty check completely in this case of CheckArrayOrEmpty since CellUse only accepts SpecCell | SpecEmpty.
> 
> Ditto

Fixed.
Comment 5 Yusuke Suzuki 2020-01-22 12:45:43 PST
Committed r254936: <https://trac.webkit.org/changeset/254936>