bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

SM: uniform handling of bytecodes with variable stack uses




JavaScript Engine
10 years ago
10 years ago


(Reporter: Igor Bukanov, Assigned: Igor Bukanov)


Dependency tree / graph
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)



(1 attachment)



10 years ago
Currently jsopcode.tbl defines that JSOP_LEAVEBLOCK and JSOP_LEAVEBLOCKEXPR use 0 stack slots when in reality they pop the local variable from stack. This discrepancy forces the emitter and the decompiler to special-case handling of the bytecodes and explicitly update the stack. 

Thus I suggest to set the use counts for the bytecodes to -1 so they can be handles in the same way as the rest of stack-variadic operations. This should allow to have a single helper function to get the real use counts for the variadic bytecodes based on their operands.

Comment 1

10 years ago
Created attachment 332616 [details] [diff] [review]

To ensure uniform handling of stack-variadic bytecodes, the patch changes:

-OPDEF(JSOP_LEAVEBLOCK,    200,"leaveblock",  NULL,    3,  0,  0,  0,  JOF_UINT16)
+OPDEF(JSOP_LEAVEBLOCK,    200,"leaveblock",  NULL,    3, -1,  0,  0,  JOF_UINT16)

-OPDEF(JSOP_LEAVEBLOCKEXPR,215,"leaveblockexpr",NULL,  3,  0,  0,  1,  JOF_UINT1
+OPDEF(JSOP_LEAVEBLOCKEXPR,215,"leaveblockexpr",NULL,  3, -1,  1,  1,  JOF_UINT16)

This allowed to have the single helper function, js_GetVariableStackUseLength, that used both by the emitter and the decompiler avoiding code duplication.

I aslo changed:

-OPDEF(JSOP_ENTERBLOCK,    199,"enterblock",  NULL,    3,  0,  0,  0,  JOF_OBJECT)
+OPDEF(JSOP_ENTERBLOCK,    199,"enterblock",  NULL,    3,  0, -1,  0,  JOF_OBJECT)

This way UpdateDepth in jsemit.cpp could check for a negative ndef and update the stack based on the block object.

Yet another change is:  

-OPDEF(JSOP_RETSUB,    115,"retsub",     NULL,         1,  0,  0,  0,  JOF_BYTE)
+OPDEF(JSOP_RETSUB,    115,"retsub",     NULL,         1,  2,  0,  0,  JOF_BYTE)

-OPDEF(JSOP_FINALLY,     134,"finally",    NULL,       1,  0,  0,  0,  JOF_BYTE)
+OPDEF(JSOP_FINALLY,     134,"finally",    NULL,       1,  0,  2,  0,  JOF_BYTE)

This allowed to remove any special handling of JSOP_FINALLY and JSOP_RETSUB when updating the stack depth.
Attachment #332616 - Flags: review?(mrbkap)


10 years ago
Attachment #332616 - Flags: review?(brendan)

Comment 2

10 years ago
fixing proper dependency
Blocks: 448828
No longer blocks: 446386
Comment on attachment 332616 [details] [diff] [review]

Sorry for the delay in review.
Attachment #332616 - Flags: review?(mrbkap) → review+
Ooh, would this fix our undiagnosed Windows-only bug in turning on JSOP_NEWARRAY or JSOP_CONCAT?

Comment on attachment 332616 [details] [diff] [review]

>+ * Find the number of stack slots that the bytecode uses when JSCodeSpec.nuses
>+ * is -1.

Suggest "Find the number of stack slots used by a variadic opcode such as JSOP_CALL or JSOP_NEWARRAY (for such ops, JSCodeSpec.nuses is -1)."

r=me with that, thanks.

Attachment #332616 - Flags: review?(brendan) → review+

Comment 6

10 years ago
landed - http://hg.mozilla.org/mozilla-central/rev/774fe6f69796
Last Resolved: 10 years ago
Resolution: --- → FIXED


10 years ago
Flags: in-testsuite-
Flags: in-litmus-
Blocks: 453492
No longer blocks: 453492
Depends on: 453492
You need to log in before you can comment on or make changes to this bug.