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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

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.
(Assignee)

Comment 1

10 years ago
Created attachment 332616 [details] [diff] [review]
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.
Attachment #332616 - Flags: review?(mrbkap)
(Assignee)

Updated

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

Comment 2

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

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?

/be
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
Attachment #332616 - Flags: review?(brendan) → review+
(Assignee)

Comment 6

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

Updated

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.