Summary: | B3 should have global store elimination | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||||
Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue, keith_miller, mark.lam, msaboff, saam | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Bug Depends on: | 154782 | ||||||||||||||
Bug Blocks: | 154319 | ||||||||||||||
Attachments: |
|
Description
Filip Pizlo
2016-02-24 14:54:35 PST
The main issue here is doing backward dominators: if we have @s = Store(@x, @p) and @t = Store(@y, @p) then we can eliminate @s if it is backward dominated by @t and there does not exist a path from @s to @t that could observe the store. On the other hand, maybe you could just do this by forward flow. If you do forward flow from @s and we find any number of @t's without ever hitting a side exit, return, read or write, then we can eliminate @s. Created attachment 272151 [details]
things
Created attachment 272227 [details]
better approach
Created attachment 272240 [details]
it compiles
Created attachment 272461 [details]
maybe the patch
Still running tests.
Attachment 272461 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:296: Place brace on its own line for function definitions. [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.
Created attachment 272470 [details]
the patch
Attachment 272470 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:296: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 1 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 272470 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=272470&action=review > Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:108 > + for (Value* candidateValue : *matches) { MemoryValue* candidateValue? > Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:109 > + if (MemoryValue* candidateMemory = candidateValue->as<MemoryValue>()) { When can this be false? (In reply to comment #9) > Comment on attachment 272470 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272470&action=review > > > Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:108 > > + for (Value* candidateValue : *matches) { > > MemoryValue* candidateValue? Something in our list could have been turned into Identity. We do like this: 1) Summarize things in each block before doing any changes. 2) Do changes (i.e. turn things into Identity's) and look for things in the summaries. The things that we turn into Identity's are also the things we put into the summaries in (1). This data structure manages summaries, so it will see Identity's. Identity does not use MemoryValue (it's just a plain Value), so MemoryValue* here would be wrong. > > > Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:109 > > + if (MemoryValue* candidateMemory = candidateValue->as<MemoryValue>()) { > > When can this be false? See above! :-) Landed in http://trac.webkit.org/changeset/197366 |