Closed Bug 1054340 Opened 6 years ago Closed 6 years ago
Offset MIR instructions
As noted in bug 1004831 (https://bugzilla.mozilla.org/show_bug.cgi?id=1004831#c83), the MPcOffset instructions are cluttering up MIR. There is no need for them and they can be eliminated altogether.
This strips all the PcOffset IR instructions, including MPcOffset and LPcOffset. An alternative (and much smaller) patch is to leave the instructions in, and just delete the two lines in IonBuilder.cpp that add the MPcOffset code. If you think the lighter approach is better let me know and I'll post a patch for that. Try run for this patch: https://tbpl.mozilla.org/?tree=Try&rev=277db1b02545
Attachment #8473731 - Flags: review?(jdemooij)
Comment on attachment 8473731 [details] [diff] [review] fix-pc-offsets.patch Review of attachment 8473731 [details] [diff] [review]: ----------------------------------------------------------------- Very logical patch and I should have seen this during reviewing. I asked it during the first review if this would also happen during debug, but forgot to follow-up on it. It is cool we actually don't need PcOffset. But IIRC atm this means we don't track anything for now, right? (I see setTrackedSite nowhere used). So this still needs to get added, right. Or am I missing something.
Attachment #8473731 - Flags: review?(jdemooij) → review+
(In reply to Hannes Verschore [:h4writer] from comment #2) > (I see setTrackedSite nowhere used). So this still needs to get added, > right. Or am I missing something. See |MBasicBlock::add| in MIRGraph.cpp. Whenever an instruction gets added it gets assigned the current bytecode site from the compiler. That behaviour was actually introduced by a previous patch on an unrelated profiler bug because we needed to track bytecode sites properly for ICs.
Assignee: nobody → kvijayan
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.