449494 - SM: uniform handling of bytecodes with variable stack uses

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

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

[Igor Bukanov]

Attachment: v1

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
6)
+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.

Comment 2

[Igor Bukanov]

fixing proper dependency

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

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

Sorry for the delay in review.

Comment 4

[Brendan Eich [:brendan]]

Ooh, would this fix our undiagnosed Windows-only bug in turning on JSOP_NEWARRAY or JSOP_CONCAT?

/be

Comment 5

[Brendan Eich [:brendan]]

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

>+/*
>+ * 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.

/be

Comment 6

[Igor Bukanov]

landed - http://hg.mozilla.org/mozilla-central/rev/774fe6f69796