Hi Joe,

On Oct 12, 2015, at 8:53 AM, joe darcy <[email protected]> wrote:

> Please review the next step of porting fdlibm to Java, the cbrt method:
> 
>    JDK-8136799 Port fdlibm cbrt to Java
> http://cr.openjdk.java.net/~darcy/8136799.3/

FdLibm.java:
* I assume that the diffs other than those of the Cbrt class itself are 
artifacts of webrev creation and no other (significant) changes are present 
elsewhere in the patch.
* Based on code inspection, the cbrt port itself appears to be accurate.

CubeRootTests.java:
* line 487: “hypot” -> “cbrt”
* line 504: probably need to insert a blank line after this line and possibly 
remove the blank line at 507.

> A few points of note about the port, there is a transliteration port in the 
> test area for comparison purposes. I also ran these tests against an 8 update 
> release where fdlibm is in C; this helps verify the transliteration port is 
> correct. The small updates to constrain the @build directive in the tests to 
> just the random number library allow the regression tests to be run 
> out-of-the-box against JDK 8 as well as JDK 9.

That all sounds good.

Reviewed: +1.

Brian

Reply via email to