Closed Bug 1455280 Opened 2 years ago Closed 2 years ago

Remove flow sensitive alias analysis

Categories

(Core :: JavaScript Engine: JIT, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Tracking Status
firefox61 --- affected

People

(Reporter: mgaudet, Assigned: mgaudet)

References

Details

Attachments

(1 file, 2 obsolete files)

It's been dormant for a year, has zero coverage, and is not being tested right now.
Assignee: nobody → mgaudet
Comment on attachment 8969302 [details] [diff] [review]
Remove FlowAliasAnalysis

Review of attachment 8969302 [details] [diff] [review]:
-----------------------------------------------------------------

\o/ Thanks for doing this!

Maybe as a follow-up we can merge AliasAnalysisShared back into AliasAnalysis? That will also make it easier to optimize the code for the C++ compiler when everything is in a single AliasAnalysis.cpp file. We can also devirtualize the analyze method :)
Attachment #8969302 - Flags: review?(jdemooij) → review+
Benjamin, this will probably break AWFY because it removes the alias analysis flag from the shell. We should just remove the flow alias analysis from AWFY.

Matthew, we should probably wait for that to avoid unnecessary AWFY downtime.
Flags: needinfo?(bbouvier)
Blocks: 1455770
Oh the fuzzers also want to know about the --ion-aa shell flag removal.

Maybe we could keep it as a no-op if that makes things easier.
Flags: needinfo?(nth10sd)
Flags: needinfo?(choller)
Keeping things as a no-op around at least for a while helps bisection of older bugs. Maybe we should keep them around for a while, remove them from all fuzzing configs, then at another point in time, remove a bunch of the no-op flags in one swoop.
Flags: needinfo?(choller)
Pretty much what :decoder said. :)
Flags: needinfo?(nth10sd)
Depends on: 1456155
Opened bug 1456155; it'd be nice to wait for a few hours before this bug here gets landed (see comment there).
Flags: needinfo?(bbouvier)
Ok so let's just keep this shell flag as a no-op for now, so we don't break fuzzing.
The option is left behind to ease transition for fuzzers.
Attachment #8969302 - Attachment is obsolete: true
Comment on attachment 8970655 [details] [diff] [review]
Remove FlowAliasAnalysis implementation

Carrying Jan's r+
Attachment #8970655 - Flags: review+
Keywords: leave-open
Attachment #8970655 - Attachment is obsolete: true
Attachment #8970663 - Flags: review+
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3fc868b50a2
Remove FlowAliasAnalysis implementation r=jandem
I'm going to close this, and open a new bug to remove the option.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Blocks: 1460883
You need to log in before you can comment on or make changes to this bug.