930708 - Add DoubleEqualsInt32 which accepts negative zero

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Dan Gohman [:sunfish]]

Attachment: range-double-equals-int32.patch

The attached patch adds a DoubleEqualsInt32 utility, which is similar to DoubleIsInt32 except that it considers negative zero to be equal to zero. This has a variety of uses, include range analysis' beta node insertion, and several places which were doing a manual conversion with an equality check.

DoubleEqualsInt32 is better than manual code because converting a double to int32 can technically invoke undefined behavior --- see the existing XXX comment in DoubleIsInt32. With this patch, this is done in one place, with said comment, rather than in several places, which is an improvement.

Comment 1

[Nicolas B. Pierron [:nbp — off until 29-09]]

Comment on attachment 821919 [details] [diff] [review]
range-double-equals-int32.patch

Review of attachment 821919 [details] [diff] [review]:
-----------------------------------------------------------------

Sounds good to me.
I put Waldo in the feedback loop.

Comment 2

[Jeff Walden [:Waldo]]

Comment on attachment 821919 [details] [diff] [review]
range-double-equals-int32.patch

Review of attachment 821919 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsmath.cpp
@@ +40,5 @@
>  using namespace js;
>  
>  using mozilla::Abs;
>  using mozilla::DoubleIsInt32;
> +using mozilla::DoubleEqualsInt32;

E before I, so one line earlier.

Comment 3

[Dan Gohman [:sunfish]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #2)
> Comment on attachment 821919 [details] [diff] [review]
> range-double-equals-int32.patch
> 
> Review of attachment 821919 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jsmath.cpp
> @@ +40,5 @@
> >  using namespace js;
> >  
> >  using mozilla::Abs;
> >  using mozilla::DoubleIsInt32;
> > +using mozilla::DoubleEqualsInt32;
> 
> E before I, so one line earlier.

Done.

https://hg.mozilla.org/integration/mozilla-inbound/rev/592b05772531

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/592b05772531