Bug 204536

Summary: Use LLInt profiling to rule out generating an IC for get_by_val
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, fpizlo, ggaren, gskachkov, guijemont, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
ysuzuki: review+
patch for landing none

Saam Barati
Reported 2019-11-22 14:08:26 PST
...
Attachments
patch (23.31 KB, patch)
2019-11-22 17:34 PST, Saam Barati 		ysuzuki: review+
DetailsFormatted DiffDiff
patch for landing (30.46 KB, patch)
2019-11-22 18:27 PST, Saam Barati 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2019-11-22 17:34:49 PST
Created attachment 384216 [details] patch
Yusuke Suzuki
Comment 2 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.
Saam Barati
Comment 3 2019-11-22 18:27:04 PST
Created attachment 384221 [details] patch for landing
Saam Barati
Comment 4 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
WebKit Commit Bot
Comment 5 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>
WebKit Commit Bot
Comment 6 2019-11-22 20:45:23 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7 2019-11-22 20:46:19 PST
<rdar://problem/57448652>
Saam Barati
Comment 8 2019-11-25 18:13:56 PST
32-bit build fix in: https://trac.webkit.org/changeset/252871/webkit
Note You need to log in before you can comment on or make changes to this bug.