Closed Bug 592631 Opened 9 years ago Closed 9 years ago

JM: fast path for x | 0

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 1 obsolete file)

Attached patch Patch v1 (obsolete) — Splinter Review
Consider this micro-benchmark:
-----
function g(a) {
    var res;
    var t0 = new Date;
    
    for(var i=0; i<10000000; i++) {
        var x = i + 0.2;
        res = x | 0;
    }
    print(new Date - t0);
    print(res);
    return res;
}
-----
Performance drops from 785 ms. to 195 ms., about as fast as V8. Which means it's actually faster, because V8 has less loop overhead.

It's quite some code though.. Is there a more efficient way to do this?
Attachment #471075 - Flags: review?(dvander)
Btw, this patch also optimizes x << 0.
Status: NEW → ASSIGNED
Attached patch Patch v2Splinter Review
Woops, use copyDataIntoReg instead of ownRegForData.
Attachment #471075 - Attachment is obsolete: true
Attachment #471086 - Flags: review?(dvander)
Attachment #471075 - Flags: review?(dvander)
Comment on attachment 471086 [details] [diff] [review]
Patch v2


>+    bool lhsIntOrDouble = !(lhs->isNotType(JSVAL_TYPE_DOUBLE) && 
>+        lhs->isNotType(JSVAL_TYPE_INT32));

Nit: second line should indent to the !

>+        rhs->isType(JSVAL_TYPE_INT32) && rhs->getValue().toInt32() == 0 &&
>+        (op == JSOP_BITOR || op == JSOP_LSH)) {
>+        

Nit: No newline here, if it looks too crowded you can put the { on the next line.

Nice work, r=me
Attachment #471086 - Flags: review?(dvander) → review+
http://hg.mozilla.org/tracemonkey/rev/ae6ca63c34c3
Assignee: general → jandemooij
Keywords: checkin-needed
Whiteboard: fixed-in-tracemonkey
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.