| Summary: | DFG::LICMPhase should attempt to hoist edge type checks if hoisting the whole node fails | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||
| Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | benjamin, commit-queue, ews-watchlist, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, saam, ticaiolima, webkit-bug-importer, ysuzuki | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Filip Pizlo
2015-05-02 10:26:52 PDT
*** Bug 185795 has been marked as a duplicate of this bug. *** 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.
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 on attachment 340832 [details]
patch
This doesn't do check hoisting on nodes that write. That's wrong...
Created attachment 340865 [details]
patch
Benchmarks seem neutral. It may be a speedup in one or two subtests in Octane. Going to run them again to verify. (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 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 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 on attachment 340865 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=340865&action=review > Source/JavaScriptCore/dfg/DFGUseKind.h:310 > + case Int32Use: > + case KnownInt32Use: > + case AnyIntUse: > + case NumberUse: Why do we say that these crash when the input is empty? Int32 check will only pass if the input is non-empty. Probably ditto for a lot of these. Comment on attachment 340865 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=340865&action=review >> Source/JavaScriptCore/dfg/DFGUseKind.h:310 >> + case NumberUse: > > Why do we say that these crash when the input is empty? > > Int32 check will only pass if the input is non-empty. Probably ditto for a lot of these. We're returning false here, so the opposite of crashing :-) Comment on attachment 340865 [details] patch Clearing flags on attachment: 340865 Committed r232075: <https://trac.webkit.org/changeset/232075> All reviewed patches have been landed. Closing bug. |