Last Comment Bug 647628 - Remove SKIP_POP_AFTER_SET
: Remove SKIP_POP_AFTER_SET
Status: RESOLVED FIXED
:
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]
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

Description 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 Luke Wagner [:luke] 2011-04-03 21:02:48 PDT
Yeah, opcode fusing has been no end of troubles in the tracer.
Comment 2 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 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 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 Marco Bonardo [::mak] 2011-11-28 05:20:10 PST
https://hg.mozilla.org/mozilla-central/rev/bc7ff681a4ae

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