Last Comment Bug 647628 - Remove SKIP_POP_AFTER_SET
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
-- normal (vote)
: mozilla11
Assigned To: Tom Schuster [:evilpie]
: Jason Orendorff [:jorendorff]
Depends on: 698201
Blocks: 647620
  Show dependency treegraph
Reported: 2011-04-03 19:01 PDT by Brian Hackett (:bhackett)
Modified: 2011-11-28 05:20 PST (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

remove the marcos (7.37 KB, patch)
2011-10-28 13:35 PDT, Tom Schuster [:evilpie]
bhackett1024: review+
Details | Diff | Splinter Review

Description User image Brian Hackett (:bhackett) 2011-04-03 19:01:43 PDT
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 User image Luke Wagner [:luke] 2011-04-03 21:02:48 PDT
Yeah, opcode fusing has been no end of troubles in the tracer.
Comment 2 User image Tom Schuster [:evilpie] 2011-10-28 13:35:32 PDT
Created attachment 570344 [details] [diff] [review]
remove the marcos

I also included the changes to jsop_toid, hope this is okay.
Comment 3 User image Brian Hackett (:bhackett) 2011-10-28 14:17:31 PDT
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.
Comment 4 User image Tom Schuster [:evilpie] 2011-11-25 04:24:32 PST
Comment on attachment 570344 [details] [diff] [review]
remove the marcos

So let's try again
Comment 6 User image Marco Bonardo [::mak] 2011-11-28 05:20:10 PST

Note You need to log in before you can comment on or make changes to this bug.