Closed Bug 804636 Opened 8 years ago Closed 8 years ago

Consider decomposing LOCAL/ARG inc/dec ops

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(4 files, 1 obsolete file)

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).
(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.
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)
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+
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+
I will check some benchmarks before landing this.
Attachment #675605 - Flags: review?(bhackett1024)
Attachment #675605 - Flags: review?(bhackett1024) → review+
Whiteboard: [leave open]
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
Closed: 8 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.