Summary: | B3ReduceStrength::simplifyCFG() could do a lot more on each iteration | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robin Morisset <rmorisset> | ||||||
Component: | JavaScriptCore | Assignee: | Robin Morisset <rmorisset> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 194222 | ||||||||
Attachments: |
|
Description
Robin Morisset
2019-02-09 14:34:21 PST
Iterating on the blocks in postOrder reduces the number of iterations on average on JetStream2 from 3.35 to 3.24: Iterations Baseline PostOrder +Loop CFGa/c 1 164 165 167 2 554 557 551 3 1690 1931 1936 4 1626 1556 1545 5 309 159 167 6 46 24 26 7 9 7 7 8 1 0 0 9 0 0 0 10 0 0 0 On the other hand while looping CFGa and CFGc is profitable in isolation (numbers not shown above), the last column shows it brings nothing on top of iterating on the blocks in post-order. Created attachment 361614 [details]
Patch
Comment on attachment 361614 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361614&action=review > Source/JavaScriptCore/ChangeLog:8 > + B3ReduceStrength::simplifyCFG() does three optimizations (which I will call CFGa, CFGb and CFGc): nit: just labeling these "A"/"B"/"C" is easier for readability IMO > Source/JavaScriptCore/ChangeLog:9 > + - CFGa makes any terminal that points to a block empty except for a jump point to that jump instead. This sentence confuses me. What's making a terminal "empty"? > Source/JavaScriptCore/ChangeLog:11 > + - CFGc inlines any block with a single predecessor that ends in a jump into the end of that predecessor. Doesn't this require the predecessor to have a single successor, and the successor to have a single predecessor? If I have a single predecessor, but that predecessor has two successors, I can't do this transformation. Created attachment 361691 [details]
Patch
Thanks for the review. (In reply to Saam Barati from comment #3) > Comment on attachment 361614 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=361614&action=review > > > Source/JavaScriptCore/ChangeLog:8 > > + B3ReduceStrength::simplifyCFG() does three optimizations (which I will call CFGa, CFGb and CFGc): > > nit: just labeling these "A"/"B"/"C" is easier for readability IMO Done. > > Source/JavaScriptCore/ChangeLog:9 > > + - CFGa makes any terminal that points to a block empty except for a jump point to that jump instead. > > This sentence confuses me. What's making a terminal "empty"? Sorry, I meant "a block that is empty". I fixed it, and tried to clarify the rest of the sentence. > > > Source/JavaScriptCore/ChangeLog:11 > > + - CFGc inlines any block with a single predecessor that ends in a jump into the end of that predecessor. > > Doesn't this require the predecessor to have a single successor, and the > successor to have a single predecessor? If I have a single predecessor, but > that predecessor has two successors, I can't do this transformation. I did not realize that my sentence was ambiguous: "that ends in a jump" was meant to apply to the predecessor (so that it indeed has a single successor). I've reworded it, please tell me if it is clearer. Comment on attachment 361691 [details]
Patch
r=me
Comment on attachment 361691 [details] Patch Clearing flags on attachment: 361691 Committed r241768: <https://trac.webkit.org/changeset/241768> All reviewed patches have been landed. Closing bug. |