Closed
Bug 1054340
Opened 10 years ago
Closed 10 years ago
Remove MPcOffset MIR instructions
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: djvj, Assigned: djvj)
References
Details
Attachments
(1 file)
8.66 KB,
patch
|
h4writer
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
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 2•10 years ago
|
||
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+
Assignee | ||
Comment 3•10 years ago
|
||
(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 | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e2e5dffbb2e
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7e2e5dffbb2e
Assignee: nobody → kvijayan
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Updated•10 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•