Bug 154658 – B3 should have global store elimination

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

...

[Filip Pizlo]

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.

[Filip Pizlo]

Created attachment 272151 [details]
things

[Filip Pizlo]

Created attachment 272227 [details]
better approach

[Filip Pizlo]

Created attachment 272240 [details]
it compiles

[Filip Pizlo]

Created attachment 272461 [details]
maybe the patch

Still running tests.

[WebKit Commit Bot]

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.

[Filip Pizlo]

Created attachment 272470 [details]
the patch

[WebKit Commit Bot]

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.

[Benjamin Poulain]

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?

[Filip Pizlo]

(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! :-)

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/197366