605858 - TM: Trace inc() on all primitives

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

This came up for undefined on jsperf.com in bug 605486, but just doing all primitives is easy enough.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Trace inc() for all primitive values, not just numbers.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 484715 [details] [diff] [review]
Trace inc() for all primitive values, not just numbers.

I used insImmD in the null case because that's what the number case used to do.... but should they both use insImmI?  Or is that handled elsewhere?

Comment 3

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

Comment on attachment 484715 [details] [diff] [review]
Trace inc() for all primitive values, not just numbers.

insImmD is right, integers shouldn't leak into the LIR (doubles are promoted to integers in a forward pipeline).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 484715 [details] [diff] [review]
Trace inc() for all primitive values, not just numbers.

Requesting approval.  This is pretty safe, and is needed (barring site changes) for jsperf.com to trace.

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 484715 [details] [diff] [review]
Trace inc() for all primitive values, not just numbers.

>+    // XXXbz can we manage to do this for objects with an imacro?  Or
>+    // will that be unsafe for some of our callers?

No need to ask and we avoid XXX comments now in favor of FIXME: bug NNNNNN comments. This one is completely fixable with imacrology, so please file a future bug.

>+    if (!v.isPrimitive())
>+        RETURN_STOP("can only inc primitives");

Uber-nit: "can inc primitives only".

Nice patch. bz, you need to do a JM patch too some time, get both decoder rings ;-).

/be

Comment 6

[Boris Zbarsky [:bzbarsky]]

(In reply to comment #5)
> No need to ask and we avoid XXX comments now in favor of FIXME: bug NNNNNN

Done.  Bug 606071.

> Uber-nit: "can inc primitives only".

Done.

> Nice patch. bz, you need to do a JM patch too some time

I looked at the pic code some; got lost and gave up for the time being.  ;)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Merge to tip and all the rewriting

Comment 8

[Boris Zbarsky [:bzbarsky]]

Robert, want to either approve or deny so I can stop worrying about merging this?  ;)

Comment 9

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/tracemonkey/rev/19f70f8c2b88

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/19f70f8c2b88