1508521 - Increment/decrement operations

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Robin Templeton [:terpri]]

Introduce increment and decrement JS ops for use by the corresponding operators. Currently the increment and decrement operators are expanded by the frontend to a sequence of arithmetic instructions, complicating BigInt support. This patch defines new opcodes for increment and decrement, treating them as new unary arithmetic operators.

Comment 1

[Robin Templeton [:terpri]]

Attachment: bug 1508521 - Introduce new increment and decrement operations.

Comment 2

[Kannan Vijayan [:djvj]]

Good to see this work progressing.  Very excited to be able to have some real integers in JS :)

Dropped in to triage this bug and give a heads up:

I previously tried to optimize the local-variable '++' operator (which desugared to GETLOCAL, POS, DUP, ONE, ADD, SETLOCAL, POP) into a single INCRLOCAL op.

After all was said and done, I noticed that there was some slight but notable regressions in the Speedometer benchmark.  When I investigated, I found that IonMonkey's codegen's type-specialization behaviour depended on ICs associated with specific internal opcodes (e.g. information gleaned from the IC for POS, and the IC for ADD).  There were other places where pattern matching against specific ops in that sequence was being used.

I fixed all of those and the perf regression got better, but there was still some residual, slight perf regression.

Just want to give a heads up on this bug to keep an eye out for these perf issues later on.  If it's small enough it still makes sense to land (as hot-loop performance is less important to web code than good handling of objects and nominal polymorphism).

Comment 3

[Andy Wingo [:wingo]]

Hi! I picked up this patch from Robin and banged it into shape, with an IC for the INC and DEC opcodes. Perf results here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=95b175cee06552ebb1218da0cc0e518b829050fe&group_state=expanded&selectedJob=220560775

Nothing stuck out for me but I am not a treeherder expert.

Comment 4

[Daniel Varga [:dvarga]]

Hi! Can you please get the review for the current diff?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/560290f26351
Introduce new increment and decrement operations. r=jandem

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/560290f26351

Comment 7

[Florin Strugariu [:Bebe]]

This generated a performance regresiion see bug 1521080

https://treeherder.mozilla.org/perf.html#/alerts?id=18792