Hi Laurent,

I take it the comments are copied from StrictMath and so the code may not always follow their guidelines (if you set CHECK_NAN to false for instance).

I notice that CHECK_NAN is false, but you get the right result below. Do you also get the right result for the Floor function?

You might want to add a unit test case for this.  Have you done that before?

Also, if you have write access to that bug, feel free to add your comments. Were you looking for a more direct dialog? I could track someone down for you to discuss with. I've usually talked with Joe Darcy about these things, but let me see if you has spare cycles for a conversation first if you want to go that route...

                        ...jim

On 6/19/15 2:06 PM, Laurent Bourgès wrote:
Jim,

Here is a new webrev to improve FloatMath:
http://cr.openjdk.java.net/~lbourges/marlin/marlin-s3.0/

I tested it with following float values:
floats       = [-2.13422758E9, -1.37992608E8, -134758.4, -131.5, -17.2,
-1.9, -0.9, -1.0E-4, -1.0E-8, -1.0E-23, -100.0, -3.0, -1.0, -0.0, 0.0,
0.0, 1.0, 3.0, 100.0, 131.5, 17.2, 1.9, 0.9, 1.0E-4, 1.0E-8, 1.0E-23,
2.13422758E9, 1.37992608E8, 134758.4, NaN, -Infinity, -5.0E20, 5.0E20,
Infinity]

strictMathCeil   = [-2134227584, -137992608, -134758, -131, -17, -1, 0,
0, 0, 0, -100, -3, -1, 0, 0, 0, 1, 3, 100, 132, 18, 2, 1, 1, 1, 1,
2134227584, 137992608, 134759, 0, -2147483648, -2147483648, 2147483647,
2147483647]

floatMathCeil    = [-2134227584, -137992608, -134758, -131, -17, -1, 0,
0, 0, 0, -100, -3, -1, 0, 0, 0, 1, 3, 100, 132, 18, 2, 1, 1, 1, 1,
2134227584, 137992608, 134759, 0, -2147483648, -2147483648, 2147483647,
2147483647]

The previous implementation was not correct with +/-Infinity and NaN.


Please note I found one bug very similar to my changes:
https://bugs.openjdk.java.net/browse/JDK-8091651

Do you think it is worth to discuss ceil/floor(float) optimizations with
core-libs ?

Laurent

Reply via email to