Closed Bug 1054340 Opened 6 years ago Closed 6 years ago

Remove MPcOffset MIR instructions

Categories

(Core :: JavaScript Engine: JIT, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: djvj, Assigned: djvj)

References

Details

Attachments

(1 file)

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.
Blocks: 1004831
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.
https://hg.mozilla.org/mozilla-central/rev/7e2e5dffbb2e
Assignee: nobody → kvijayan
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Flags: qe-verify-
Depends on: 1096138
You need to log in before you can comment on or make changes to this bug.