Consider decomposing LOCAL/ARG inc/dec ops

RESOLVED FIXED in mozilla19

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

7 years ago
Handling these in the JITs is quite complicated, especially with IonMonkey: compilation aborts if the input is not known to be int32/double, since we can't bailout in the middle of a MToInt32/MToDouble + MAdd sequence.

Decomposing these like the other INC/DEC ops lets us remove at least 300 lines of code, mostly from the JITs. Main downside is that it increases bytecode size (4 to 15 ops).
(Assignee)

Comment 1

7 years ago
(In reply to Jan de Mooij [:jandem] from comment #0)
> 
> Main downside is that it increases
> bytecode size (4 to 15 ops).

Eh s/ops/bytes of course.
(Assignee)

Comment 2

7 years ago
15 files changed, 31 insertions(+), 548 deletions(-)

Will post another patch to remove the loop test analysis from LoopState, since this patch makes it useless.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #674401 - Flags: review?(bhackett1024)
(Assignee)

Updated

7 years ago
(Assignee)

Comment 3

7 years ago
Just some minor changes.
Attachment #674401 - Attachment is obsolete: true
Attachment #674401 - Flags: review?(bhackett1024)
Attachment #675156 - Flags: review?(bhackett1024)
Comment on attachment 675156 [details] [diff] [review]
Part 1: Decompose LOCAL/ARG inc/dec, v2

Review of attachment 675156 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.  Can you make sure though that this doesn't break the decompiler, so that the JIT inspector is still usable?  Unfortunately I think the inspector is currently the only way to exercise the decompiler, so you'd need to use the addon a bit and make sure it behaves right on scripts with inc/dec ops.
Attachment #675156 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 5

7 years ago
I managed to make the decompiler work in the shell again, this patch fixes it.
Attachment #675300 - Flags: review?(bhackett1024)
Attachment #675300 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 6

7 years ago
I will check some benchmarks before landing this.
Attachment #675605 - Flags: review?(bhackett1024)
Attachment #675605 - Flags: review?(bhackett1024) → review+
(Assignee)

Updated

7 years ago
Whiteboard: [leave open]
(Assignee)

Comment 8

7 years ago
Removes another 65 lines I forgot in the first patch: TypeOracle::incslot and special handling for inc/dec ops in TypeDynamicResult.
Attachment #675836 - Flags: review?(dvander)
Attachment #675836 - Flags: review?(dvander) → review+
https://hg.mozilla.org/mozilla-central/rev/a3dcaf02c5e5
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Depends on: 807527
Depends on: 807528
You need to log in before you can comment on or make changes to this bug.