462103 - TM: We don't trace some variants of string + other type

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Andreas Gal :gal]

Attachment: patch

Comment 1

[Brendan Eich [:brendan]]

Attachment: reorder booleanAtoms and typeAtoms, pick nits and perf buglets

I can land this if it's good -- let me know if I missed any nits. Interdiff to see what changed.

/be

Comment 2

[Andreas Gal :gal]

Comment on attachment 345244 [details] [diff] [review]
reorder booleanAtoms and typeAtoms, pick nits and perf buglets

Relying on booleanAtoms overflowing into typeAtoms is going to come back and bite you one day. And I will be there and say I told you so ;)

Comment 3

[Brendan Eich [:brendan]]

(In reply to comment #2)
> (From update of attachment 345244 [details] [diff] [review])
> Relying on booleanAtoms overflowing into typeAtoms is going to come back and
> bite you one day. And I will be there and say I told you so ;)

Only if you remove the static asserts already there, and ones I'm adding in the next patch -- which happen to cover your habit of hardcoding 2 for JSVAL_TO_BOOLEAN(JSVAL_VOID) :-P. Igor's static asserts save the day here.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: patch to commit

Comment 5

[Brendan Eich [:brendan]]

Fixed in tm:

http://hg.mozilla.org/tracemonkey/rev/fb35f0d7e0c0

Hope this makes the m-c train.

/be

Comment 6

[Ed Lee :Mardak]

Comment on attachment 345246 [details] [diff] [review]
patch to commit

>+++ b/js/src/jstracer.cpp
>@@ -1934,17 +1934,17 @@ TraceRecorder::checkType(jsval& v, uint8
>-        /* We sink i2f conversions into the side exit, but at the loop edge we have to make
>+        /* We sink i2f conversions into the side exit, but at the loop edge we have to
>            sure we promote back to double if at loop entry we want a double. */
Was this accidental?

Comment 7

[Brendan Eich [:brendan]]

Yes, sorry about that -- I am restoring the "make" in another patch.

/be

Comment 8

[Brendan Eich [:brendan]]

This is in m-c now.

/be

Comment 9

[Andreas Gal :gal]

My patch. I should have closed it. I have no good way to track these though, so I take only partial blame.

Comment 10

[Bob Clary [:bc] (inactive)]

added to js1_8_1/trace/trace-test.js in http://hg.mozilla.org/mozilla-central/rev/8155aba03537