Bug 224578

Summary: [JSC] DFG / FTL should inline switch_string
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: 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 mark.lam: review+

Description Yusuke Suzuki 2021-04-14 14:48:01 PDT 
After r275840, we no longer copy string table in CodeBlock! (mainly for memory saving).
This unlocks ability to inline op_switch_string in DFG / FTL.
Comment 1 Radar WebKit Bug Importer 2021-04-21 14:48:16 PDT 
<rdar://problem/76980854>
Comment 2 Yusuke Suzuki 2021-04-21 22:43:54 PDT 
Created attachment 426776 [details]
Patch
Comment 3 Mark Lam 2021-04-22 00:10:10 PDT 
Comment on attachment 426776 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:10
> +        This allows DFG / FTL to inline CodeBlock which includes op_switch_string. We were not able

I suggest "were previously not able".

> Source/JavaScriptCore/ChangeLog:13
> +        1. We handle StringJumpTable / UnlinkedStringJumpTable in the same way to SimpleJumpTable / UnlinkedSimpleJumpTable.

/same way to/same way as/.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:16748
>          LValue branchOffset = vmCall(

Can you rename this node to `branchIndex` to match `operationSwitchStringAndGetIndex`?
Comment 4 Yusuke Suzuki 2021-04-22 00:12:46 PDT 
Comment on attachment 426776 [details]
Patch

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

>> Source/JavaScriptCore/ChangeLog:10
>> +        This allows DFG / FTL to inline CodeBlock which includes op_switch_string. We were not able
> 
> I suggest "were previously not able".

Fixed.

>> Source/JavaScriptCore/ChangeLog:13
>> +        1. We handle StringJumpTable / UnlinkedStringJumpTable in the same way to SimpleJumpTable / UnlinkedSimpleJumpTable.
> 
> /same way to/same way as/.

Fixed.

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:16748
>>          LValue branchOffset = vmCall(
> 
> Can you rename this node to `branchIndex` to match `operationSwitchStringAndGetIndex`?

Oops, fixed.
Comment 5 Yusuke Suzuki 2021-04-22 01:27:53 PDT 
Committed r276427 (236890@main): <https://commits.webkit.org/236890@main>