Closed Bug 1460883 Opened 5 years ago Closed 5 years ago

Remove flow sensitive alias analysis option


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




Tracking Status
firefox62 --- fixed


(Reporter: mgaudet, Assigned: Himanshuteli, Mentored)



(Keywords: good-first-bug)


(1 file, 1 obsolete file)

In bug 1455280 flow sensitive alias analysis was removed, but the option was left behind to simplify fuzzing and bisection. Nevertheless, at some point the option should go away.
Priority: -- → P3
I would like to work on this can you guide me
Hey, I'd love to guide you. 

First step: make sure you get a copy of the sources, and are able to build at least spidermonkey [1]. In my experience, using is the easiest way to get ready to go [2].  

So the option used to be 'ion-aa'. A very useful tool for you is, which will help you find the references to that option. If I recall correctly there are two of them. You'll just need to prepare a patch to excise them. 

I have successfully build firefox and compiled it.
If I'm correct, we need to remove the lines: 
if (op.getStringOption("ion-aa")) {}
|| !op.addStringOption('\0', "ion-aa", "flow-sensitive/flow-insensitive",
                               "Specify wheter or not to use flow sensitive Alias Analysis"
                               "(default: flow-insensitive)")
located in js/src/shell/js.cpp

Let's wait for more info.
Those look about right to me. So now, you'll do the removal, ensure things still build, then upload a patch to this bug.
Attached patch js.cpp (obsolete) — Splinter Review
Removed flow sensitive alias analysis option
(In reply to Himanshuteli from comment #5)
> Created attachment 8975864 [details] [diff] [review]
> js.cpp
> Removed flow sensitive alias analysis option

Please join #introduction on, then I will help you create a patch.
Removed flow sensitive alias analysis option. I hope this patch is correct.
Attachment #8975864 - Attachment is obsolete: true
Assignee: nobody → Himanshuteli
Comment on attachment 8979027 [details] [diff] [review]

Review of attachment 8979027 [details] [diff] [review]:

Thanks for doing this! :)
Attachment #8979027 - Flags: review+
If you are happy with the patch, add "checkin-needed" to the |Keywords| list (which you can edit by clicking Edit Bug near the top of page). A Sheriff will add my name to the commit message and checkin the patch for us.
Keywords: checkin-needed
Pushed by
Remove flow sensitive alias analysis option r=tcampbell
Keywords: checkin-needed
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
\o/ Congrats on landing your first SpiderMonkey patch!
You need to log in before you can comment on or make changes to this bug.