Bug 224578 - [JSC] DFG / FTL should inline switch_string
Summary: [JSC] DFG / FTL should inline switch_string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-14 14:48 PDT by Yusuke Suzuki
Modified: 2021-04-22 01:27 PDT (History)
7 users (show)

See Also:


Attachments
Patch (35.98 KB, patch)
2021-04-21 22:43 PDT, Yusuke Suzuki
mark.lam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>