Bug 204536 - Use LLInt profiling to rule out generating an IC for get_by_val
Summary: Use LLInt profiling to rule out generating an IC for get_by_val
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-11-22 14:08 PST by Saam Barati
Modified: 2019-11-25 18:13 PST (History)
14 users (show)

See Also:


Attachments
patch (23.31 KB, patch)
2019-11-22 17:34 PST, Saam Barati
ysuzuki: review+
Details | Formatted Diff | Diff
patch for landing (30.46 KB, patch)
2019-11-22 18:27 PST, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2019-11-22 14:08:26 PST
...
Comment 1 Saam Barati 2019-11-22 17:34:49 PST
Created attachment 384216 [details]
patch
Comment 2 Yusuke Suzuki 2019-11-22 17:49:40 PST
Comment on attachment 384216 [details]
patch

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

r=me

> Source/JavaScriptCore/bytecode/BytecodeList.rb:537
> +        seenIdentifiers: uint64_t, # Top 8 bits are the seen identifier count. Lower 56 bits are a bloom filter of seen UniquedStringImpl*.

I recommend introducing a class like GetByValHistory etc. not to use uint64_t directly.

> Source/JavaScriptCore/jit/JITPropertyAccess.cpp:97
> +    if (m_fastGetByVals.contains(currentInstruction)) {

You can count # of slowcase jumps by using `iter` (see JIT::linkAllSlowCasesForBytecodeIndex code, which iterates all slowcase jumps), and we can omit this code generation when slowcase count is 0, instead of tracking it by m_fastGetByVals.
Comment 3 Saam Barati 2019-11-22 18:27:04 PST
Created attachment 384221 [details]
patch for landing
Comment 4 Saam Barati 2019-11-22 18:28:09 PST
Comment on attachment 384216 [details]
patch

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

>> Source/JavaScriptCore/bytecode/BytecodeList.rb:537
>> +        seenIdentifiers: uint64_t, # Top 8 bits are the seen identifier count. Lower 56 bits are a bloom filter of seen UniquedStringImpl*.
> 
> I recommend introducing a class like GetByValHistory etc. not to use uint64_t directly.

nice. Done

>> Source/JavaScriptCore/jit/JITPropertyAccess.cpp:97
>> +    if (m_fastGetByVals.contains(currentInstruction)) {
> 
> You can count # of slowcase jumps by using `iter` (see JIT::linkAllSlowCasesForBytecodeIndex code, which iterates all slowcase jumps), and we can omit this code generation when slowcase count is 0, instead of tracking it by m_fastGetByVals.

nice. Done
Comment 5 WebKit Commit Bot 2019-11-22 20:45:21 PST
Comment on attachment 384221 [details]
patch for landing

Clearing flags on attachment: 384221

Committed r252825: <https://trac.webkit.org/changeset/252825>
Comment 6 WebKit Commit Bot 2019-11-22 20:45:23 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-11-22 20:46:19 PST
<rdar://problem/57448652>
Comment 8 Saam Barati 2019-11-25 18:13:56 PST
32-bit build fix in:
https://trac.webkit.org/changeset/252871/webkit