Summary: | Reduce graph size by replacing terminal nodes in blocks that have a ForceOSRExit with Unreachable | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||||||||||
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, ticaiolima, webkit-bug-importer, ysuzuki | ||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Bug Depends on: | |||||||||||||||||||
Bug Blocks: | 181447 | ||||||||||||||||||
Attachments: |
|
Description
Saam Barati
2018-01-08 15:45:56 PST
Created attachment 330759 [details]
WIP
running into some validation errors WRT liveness. Looking into it.
(In reply to Saam Barati from comment #1) > Created attachment 330759 [details] > WIP > > running into some validation errors WRT liveness. Looking into it. I think I spot the bug. Created attachment 330760 [details]
WIP
Created attachment 330772 [details]
WIP
test EWS
Created attachment 330785 [details]
patch
(In reply to Saam Barati from comment #5) > Created attachment 330785 [details] > patch Some alternatives that we could do: We can factor out the code into a helper function on Graph that does the transformation. We could then call it from elsewhere since there are other phases that introduce ForceOSRExit nodes. I didn't do this just because it seemed less profitable. We probably introduce the most ForceOSRExit nodes inside the BytecodeParser. Comment on attachment 330785 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=330785&action=review r=me. > Source/JavaScriptCore/ChangeLog:27 > + it, we could use additional inputs into this mechanism. For example, we could > + profile if a basic block ever executes inside the LLInt/Baseline, and > + remove parts of the CFG based on that. File a bug? (In reply to Keith Miller from comment #7) > Comment on attachment 330785 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=330785&action=review > > r=me. > > > Source/JavaScriptCore/ChangeLog:27 > > + it, we could use additional inputs into this mechanism. For example, we could > > + profile if a basic block ever executes inside the LLInt/Baseline, and > > + remove parts of the CFG based on that. > > File a bug? Filed: https://bugs.webkit.org/show_bug.cgi?id=181447 Comment on attachment 330785 [details] patch Clearing flags on attachment: 330785 Committed r226655: <https://trac.webkit.org/changeset/226655> All reviewed patches have been landed. Closing bug. (In reply to Radar WebKit Bug Importer from comment #11) > <rdar://problem/36383749> Not sure why, but it seems that this cause some regression in assorted-test. https://arewefastyet.com/#machine=29&view=single&suite=misc&subtest=typedobj-write-struct-field-standard It would be good if this can fix some minor issues in this patch. (In reply to Yusuke Suzuki from comment #12) > (In reply to Radar WebKit Bug Importer from comment #11) > > <rdar://problem/36383749> > > Not sure why, but it seems that this cause some regression in assorted-test. > https://arewefastyet.com/#machine=29&view=single&suite=misc&subtest=typedobj- > write-struct-field-standard > It would be good if this can fix some minor issues in this patch. It might be worth looking into. What does the test to? It seems really surprising TBH though. This got rolled out in: https://trac.webkit.org/changeset/230928/webkit But, our bots show rolling this out was a Speedometer 2 regression. Phil showed that rolling it out was a *progression* on certain tests w.r.t OSR entry. I'm going to reland a version of this that's both good for OSR entry and good for Speedometer. Created attachment 342315 [details]
WIP
Haven't had time to test it really, but this may be the patch
Attachment 342315 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:6867: One line control clauses should not use braces. [whitespace/braces] [4]
Total errors found: 1 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 342315 [details] WIP Attachment 342315 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/8093817 New failing tests: storage/domstorage/storage-close-database-on-idle.html Created attachment 342352 [details]
Archive of layout-test-results from ews200 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 342499 [details]
patch
Attachment 342499 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGGraph.cpp:933: Multi line control clauses should use braces. [whitespace/braces] [4]
Total errors found: 1 in 4 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 342499 [details]
patch
r=me.
Comment on attachment 342499 [details] patch Clearing flags on attachment: 342499 Committed r232742: <https://trac.webkit.org/changeset/232742> All reviewed patches have been landed. Closing bug. This caused crashes: https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/r232756%20(3650)/js/dom/JSON-stringify-stderr.txt (In reply to Saam Barati from comment #24) > This caused crashes: > https://build.webkit.org/results/ > Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/r232756%20(3650)/js/dom/JSON- > stringify-stderr.txt actually, it seems more likely that this broke it: https://trac.webkit.org/changeset/232741/webkit This is a 2% ARES-6 progression |