Last Comment Bug 725198 - js_DoubleToECMAUint32 implementation seems to be unnecessary
: js_DoubleToECMAUint32 implementation seems to be unnecessary
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla13
Assigned To: David Mandelin [:dmandelin]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-07 19:23 PST by Boris Zbarsky [:bz] (Out June 25-July 6)
Modified: 2012-02-09 10:17 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.89 KB, patch)
2012-02-08 17:48 PST, David Mandelin [:dmandelin]
luke: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] (Out June 25-July 6) 2012-02-07 19:23:46 PST
As far as I can tell, simply returning js_DoubleToEcmaInt32 cast to uint32_t would do the right thing.  And the conversion to Int32 is all hand-optimized and stuff, so it would be faster.  Is there a reason we're not doing that?
Comment 1 Andreas Gal :gal 2012-02-07 20:15:27 PST
If I remember correctly js_DoubleToEcma(U)Int32 is equivalent to unsigned(d) and signed(d) as long its not a corner case (finite value and such).

  double d = 0xffffffffUL;
  unsigned u = unsigned(d);
  signed i = signed(d);
  printf("%u %d %u\n", u, i, unsigned(i));

prints

4294967295 -2147483648 2147483648

So I don't think this works, but it has been a long day and I am jetlagged to pieces, so if I am on crack, I apologize in advance :)
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-02-07 20:53:45 PST
js_DoubleToEcmaInt32(double(0xffffffffUL)) would do the following:

1)  Floor the number (it's already an integer, so a no-op).
2)  Reduce mod 2^32 (already in the right range, so a no-op).
3)  Subtract 2^32, since it's >= 2^31

Hence it returns -1, unless I'm missing something.  And in fact, in JS (0xffffffff >> 0) is -1.

signed(d) for that double is not actually defined in C/C++, so the compiler can kinda make up whatever it wants.  But yours is definitely not making up -1.  ;)
Comment 3 David Mandelin [:dmandelin] 2012-02-08 10:55:15 PST
(In reply to Boris Zbarsky (:bz) from comment #0)
> As far as I can tell, simply returning js_DoubleToEcmaInt32 cast to uint32_t
> would do the right thing.  

The spec agrees with you: 9.5 says that

  for all x: ToUint32(ToInt32(x)) == ToUint32(x)

> And the conversion to Int32 is all hand-optimized
> and stuff, so it would be faster.  Is there a reason we're not doing that?

No idea. I think older editions of the spec used to be a little less clear about this, so maybe it was different, or maybe it just wasn't as clear that it works.
Comment 4 David Mandelin [:dmandelin] 2012-02-08 17:48:11 PST
Created attachment 595595 [details] [diff] [review]
Patch
Comment 5 Luke Wagner [:luke] 2012-02-08 18:16:25 PST
Comment on attachment 595595 [details] [diff] [review]
Patch

>+inline uint32_t
>+js_DoubleToECMAUint32(jsdouble d) {

{ on new line
Comment 6 David Mandelin [:dmandelin] 2012-02-08 18:39:14 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/1b0466feed19
Comment 7 Ed Morley [:emorley] 2012-02-09 10:17:00 PST
https://hg.mozilla.org/mozilla-central/rev/1b0466feed19

Note You need to log in before you can comment on or make changes to this bug.