Closed
Bug 647628
Opened 13 years ago
Closed 13 years ago
Remove SKIP_POP_AFTER_SET
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: bhackett1024, Assigned: evilpie)
References
Details
Attachments
(1 file)
7.37 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
This is an interpreter optimization that allows it to skip POP opcodes instead of pushing a value and then immediately popping it. This optimization really isn't necessary in the presence of JITs, and seems to make trace recording and other interpreter instrumentation more complicated, judging from the (scary) code snippet below from LeaveTree. /* * JSOP_SETELEM can be coalesced with a JSOP_POP in the interpeter. * Since this doesn't re-enter the recorder, the post-state snapshot * is invalid. Fix it up here. */ if (op == JSOP_SETELEM && JSOp(regs->pc[JSOP_SETELEM_LENGTH]) == JSOP_POP) { regs->sp -= js_CodeSpec[JSOP_SETELEM].nuses; regs->sp += js_CodeSpec[JSOP_SETELEM].ndefs; regs->pc += JSOP_SETELEM_LENGTH; op = JSOP_POP; }
Comment 1•13 years ago
|
||
Yeah, opcode fusing has been no end of troubles in the tracer.
Assignee | ||
Updated•13 years ago
|
Assignee: general → evilpies
Assignee | ||
Comment 2•13 years ago
|
||
I also included the changes to jsop_toid, hope this is okay.
Attachment #570344 -
Flags: review?(bhackett1024)
Reporter | ||
Comment 3•13 years ago
|
||
Comment on attachment 570344 [details] [diff] [review] remove the marcos Can you remove the SKIP_POP_AFTER_SET tracer special casting in comment 0? It might be better to wait on this bug until the tracer is completely removed, I don't know if there are other ways SKIP_POP_AFTER_SET can subtly break tracing/recording behavior.
Attachment #570344 -
Flags: review?(bhackett1024)
Assignee | ||
Comment 4•13 years ago
|
||
Comment on attachment 570344 [details] [diff] [review] remove the marcos So let's try again
Attachment #570344 -
Flags: review?(bhackett1024)
Reporter | ||
Updated•13 years ago
|
Attachment #570344 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 5•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/bc7ff681a4ae
Comment 6•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bc7ff681a4ae
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in
before you can comment on or make changes to this bug.
Description
•