Summary: | br_table is slow or it doesn't scale | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Vlad Brezae <brezaevlad> | ||||
Component: | JavaScriptCore | Assignee: | Tadeu Zagallo <tzagallo> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Enhancement | CC: | ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | Other | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Vlad Brezae
2020-05-28 23:30:40 PDT
Created attachment 400861 [details]
Patch
Comment on attachment 400861 [details]
Patch
r=me if you've verified that JetStream2 and Speedometer2 are not impacted. Can you confirm that perf is not impacted?
Comment on attachment 400861 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400861&action=review > Source/JavaScriptCore/ChangeLog:14 > + already have LICM in DFG, it should be ok to disable it for now. but B3's LICM will probably hoist things DFG LICM can't. Why not just disable it for now in Wasm code? Comment on attachment 400861 [details]
Patch
Ran perf tests and the results were neutral on JS2 and Speedometer2. Talked with Saam offline and he agreed to landing this patch and working on improving the phase in the future.
Committed r262523: <https://trac.webkit.org/changeset/262523> All reviewed patches have been landed. Closing bug and clearing flags on attachment 400861 [details]. testb3 is failing after this change, can you take a look? (In reply to Saam Barati from comment #4) > Comment on attachment 400861 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=400861&action=review > > > Source/JavaScriptCore/ChangeLog:14 > > + already have LICM in DFG, it should be ok to disable it for now. > > but B3's LICM will probably hoist things DFG LICM can't. > > Why not just disable it for now in Wasm code? I added B3’s LICM for fun and completeness. Not sad to see it disabled if it causes any issues at all whatsoever since its upside is believed to be something like zero. LICM is a cheesy optimization anyway (huge win for some benchmarks but zero effect on the overwhelming majority of loops) and the DFG’s is really good. So you ha e very few LICM opportunities and DFG finds almost all of them. The ones left to B3 are not that interesting. And in wasm I would assume that licm rarely finds something that is both profitable and legal to hoist. B3’s LICM can’t hoist checking opcodes so that immediately means no hoisting of memory accesses. Memory access hoisting is the most profitable, so that means it’s just not going to be a big perf winner. |