572081 - JM: Arithmetic Fast Paths

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Sean Stangl [:sstangl]]

Attachment: Fast-path integer addition and subtraction.

This bug tracks the implementation of arithmetic fast paths for integers and doubles. (The first iteration of JaegerMonkey lacked fast paths for multiplication and division; doubles always required a stub call.)

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 451235 [details] [diff] [review]
Fast-path integer addition and subtraction.


>+static inline bool
>+JSOpBinaryTryConstantFold(JSContext *cx, FrameState &frame, JSOp op, FrameEntry *lhs, FrameEntry *rhs)
>+{
>+    if (!lhs->isConstant() || !rhs->isConstant()) {
>+        return false;
>+    }

braces not needed

>+    RegisterID reg;

GCC gives an uninit-variable warning on Opt builds for this, it looks okay so just init it to Registers::ReturnReg or something.

>+        /* One of the values should not be a constant.
>+         * If there is a constant, force it to be in rhs. */
>+        bool swapped = false;
>+        if (lhs->isConstant()) {
>+            swapped = true;
>+            JS_ASSERT(!rhs->isConstant());
>+            FrameEntry *tmp = lhs;
>+            lhs = rhs;
>+            rhs = tmp;
>+        }
>+
>+        reg = frame.copyData(lhs);
>+        if (swapped && op == JSOP_SUB) {
>+            masm.neg32(reg);
>+            op = JSOP_ADD;
>+        }

Please verify that this works if the LHS is -2^31, the value where neg will overflow

>+        /* Int code can't possibly work. */
>+        /* TODO: Double code goes here. And changes the push below. */
>+        Jump j = masm.jump();
>+        stubcc.linkExit(j);
>+        knownPayload = false;

This should be a stub call because side exits are slow. That is, don't bother using stubcc.

Looks great, r=me w/ changes!

Comment 2

[Sean Stangl [:sstangl]]

Tested with -2^31; it correctly calculates the result.
Pushed (43607:3342a4bdfc10).

Comment 3

[Sean Stangl [:sstangl]]

Attachment: Fast

Comment 4

[Sean Stangl [:sstangl]]

Attachment: Fast-path integer multiplication.

Integer multiplication fast-paths are disabled for ARM because it is evidently not possible to check for overflow conditions.

Comment 5

[Jeff Walden [:Waldo]]

(In reply to comment #2)
> Tested with -2^31; it correctly calculates the result.

Probably we have a JS test for this already in the suite, but if we don't, please add one?

Comment 6

[Brendan Eich [:brendan]]

(In reply to comment #1)
> >+        /* One of the values should not be a constant.
> >+         * If there is a constant, force it to be in rhs. */

That's not prevailing comment style.

/be

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

ADD/SUB int fast path: 5.8% (~70ms) win on SS, 4% (~700ms) on V8
MUL int fast path: 1.3% (~13ms) win on SS, 2.3% (~380ms) on V8

Comment 8

[Sean Stangl [:sstangl]]

Attachment: Fast-path double negation.

Double fast-paths are currently a strange case, because the allocator for floating-point registers is not being pulled into the codebase pending changes to FrameState for 64-bit support. However, we want to write code supporting multiple fast-paths for various types + conversion right now, even before the allocator can be pulled.

In the process of debugging double fast-paths for arithmetic operators, I wrote a fast-path for doubles for negation, which is useful to pull to illustrate the design issues ahead in terms of functioning code.

Although this code is as naive as possible, syncing all registers and loading/storing data to the stack in all situations, measurements have it being twice as fast as the current negation code.

This patch depends on a patch implementing xorDouble().

Comment 9

[Sean Stangl [:sstangl]]

Pushed integer fast-path for multiplication (a while ago... whoops.)

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/5a2ac3332b9e

Comment 10

[Sean Stangl [:sstangl]]

dvander rewrote arithmetic fast-paths in changeset http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/51ed7672df50 .

Marking fixed.