|Summary:||DFG::LICMPhase should attempt to hoist edge type checks if hoisting the whole node fails|
|Product:||WebKit||Reporter:||Filip Pizlo <fpizlo>|
|Severity:||Normal||CC:||benjamin, commit-queue, ews-watchlist, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, sbarati, ticaiolima, webkit-bug-importer, ysuzuki|
|Version:||528+ (Nightly build)|
Description Filip Pizlo 2015-05-02 10:26:52 PDT
This usually isn't super necessary because the CPS type check hoisting - done in FixupPhase by putting checks on SetLocals - does a pretty good job. But it only covers the super common types and sometimes it will not do anything if the VariableAccessData looks sketchy. So, LICMPhase does have an opportunity here for some small additional speed-up.
Comment 1 Saam Barati 2018-05-20 22:47:13 PDT
*** Bug 185795 has been marked as a duplicate of this bug. ***
Comment 2 Saam Barati 2018-05-20 23:05:06 PDT
Created attachment 340829 [details] WIP Seems to hoist some stuff in Octane. Need to clean some things up before actually putting it up for review.
Comment 3 Saam Barati 2018-05-21 00:56:03 PDT
Created attachment 340832 [details] patch Will run some benchmarks tomorrow before landing. I've verified via logging that various checks do get hoisted in Octane.
Comment 4 Saam Barati 2018-05-21 11:10:56 PDT
Comment on attachment 340832 [details] patch This doesn't do check hoisting on nodes that write. That's wrong...
Comment 6 Saam Barati 2018-05-21 11:47:16 PDT
Benchmarks seem neutral. It may be a speedup in one or two subtests in Octane. Going to run them again to verify.
Comment 7 Saam Barati 2018-05-21 12:01:17 PDT
(In reply to Saam Barati from comment #6) > Benchmarks seem neutral. It may be a speedup in one or two subtests in > Octane. Going to run them again to verify. Seems like a 3-4% speedup on Octane/regexp. Seems neutral on most other things, with some maybe 1% speedups sprinkled about. I think the bots will be the best source of truth for this change.
Comment 8 EWS Watchlist 2018-05-21 22:05:01 PDT
Comment on attachment 340865 [details] patch Attachment 340865 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7761006 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html
Comment 9 EWS Watchlist 2018-05-21 22:05:12 PDT
Created attachment 340953 [details] Archive of layout-test-results from ews201 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews201 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 10 Filip Pizlo 2018-05-22 10:07:23 PDT
Comment 11 Saam Barati 2018-05-22 12:20:39 PDT
Comment 12 WebKit Commit Bot 2018-05-22 12:47:45 PDT
Comment on attachment 340865 [details] patch Clearing flags on attachment: 340865 Committed r232075: <https://trac.webkit.org/changeset/232075>
Comment 13 WebKit Commit Bot 2018-05-22 12:47:47 PDT
All reviewed patches have been landed. Closing bug.