Skip to content

Commit

Permalink
[WASM] Fix double/float mod bug.
Browse files Browse the repository at this point in the history
Existing logic based on narrowing to integral type is completely incorrect.
The new implementation delegates to JS for double values and adapted FreeBSD's fmod implementation for floats.

PiperOrigin-RevId: 527786033
  • Loading branch information
gkdn authored and copybara-github committed Apr 28, 2023
1 parent 9b38306 commit 7cc0f3a
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 20 deletions.
15 changes: 15 additions & 0 deletions LICENSE
Original file line number Diff line number Diff line change
Expand Up @@ -486,3 +486,18 @@ appreciated but is not required.
2. Altered source versions must be plainly marked as such, and must not be
misrepresented as being the original software.
3. This notice may not be removed or altered from any source distribution.


--------------------------------------------------------------------------------

javaemul/internal/Primitives contains code derived from FreeBSD's math library:

// ====================================================
// Copyright (C) 1993 by Sun Microsystems, Inc. All rights reserved.
//
// Developed at SunPro, a Sun Microsystems, Inc. business.
// Permission to use, copy, modify, and distribute this
// software is freely granted, provided that this notice
// is preserved.
// ====================================================

15 changes: 14 additions & 1 deletion jre/java/super-wasm/double_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,17 @@ function isValidDouble(str) {
.test(str);
}

exports = {isValidDouble};
/**
* @param {number} x
* @param {number} y
* @return {number}
*/
function dmod(x, y) {
return x % y;
}


exports = {
isValidDouble,
dmod
};
1 change: 1 addition & 0 deletions jre/java/super-wasm/j2wasm.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ function createImportObject(userImports) {
'j2wasm.CharUtils.charToUpperCase': CharUtils.charToUpperCase,
'j2wasm.ConsoleUtils.log': ConsoleUtils.log,
'j2wasm.DoubleUtils.isValidDouble': DoubleUtils.isValidDouble,
'j2wasm.DoubleUtils.dmod': DoubleUtils.dmod,
'parseFloat': parseFloat,
'performance.now': () => performance.now(),

Expand Down
129 changes: 114 additions & 15 deletions jre/java/super-wasm/javaemul/internal/Primitives.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package javaemul.internal;

import javaemul.internal.annotations.Wasm;
import jsinterop.annotations.JsMethod;

/** Static Primitive helper. */
public class Primitives {
Expand Down Expand Up @@ -195,29 +196,127 @@ public static long safeDivision(long dividend, long divisor) {
@Wasm("i64.div_s")
private static native long wasmDivision(long dividend, long divisor);

public static double dmod(double x, double y) {
if (Double.isNaN(x) || Double.isNaN(y) || Double.isInfinite(x) || y == 0.0d) {
return Double.NaN;
@JsMethod(namespace = "j2wasm.DoubleUtils")
public static native double dmod(double x, double y);

@SuppressWarnings("IdentityBinaryExpression")
public static float fmod(float x, float y) {
// Derived from FreeBSD's src/e_fmodf.c which came with this notice.
/*
* ====================================================
* Copyright (C) 1993 by Sun Microsystems, Inc. All rights reserved.
*
* Developed at SunPro, a Sun Microsystems, Inc. business.
* Permission to use, copy, modify, and distribute this
* software is freely granted, provided that this notice
* is preserved.
* ====================================================
*/

int n, hz;
int hx = Platform.floatToRawIntBits(x);
int hy = Platform.floatToRawIntBits(y);
int sx = hx & 0x80000000; /* sign of x */
hx ^= sx; /* |x| */
hy &= 0x7fffffff; /* |y| */

/* purge off exception values */
if (hy == 0 /* y=0 */
|| (hx >= 0x7f800000) /* or x not finite*/
|| (hy > 0x7f800000) /* or y is NaN */) {
return (x * y) / (x * y);
}
if (hx < hy) {
return x; /* |x|<|y| return x */
}
if (hx == hy) {
return signedZero(sx); /* |x|=|y| return x*0 */
}

if (Double.isInfinite(y) || x == 0.0d) {
return x;
/* determine ix = ilogb(x) */
int ix;
if (hx < 0x00800000) {
/* subnormal x */
ix = -126;
for (int i = (hx << 8); i > 0; i <<= 1) {
ix -= 1;
}
} else {
ix = (hx >> 23) - 127;
}
// See https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.17.3
// and https://en.wikipedia.org/wiki/Modulo_operation#Variants_of_the_definition
return x - (y * narrowDoubleToInt(x / y));
}

public static float fmod(float x, float y) {
if (Float.isNaN(x) || Float.isNaN(y) || Float.isInfinite(x) || y == 0.0f) {
return Float.NaN;
/* determine iy = ilogb(y) */
int iy;
if (hy < 0x00800000) {
/* subnormal y */
iy = -126;
for (int i = (hy << 8); i >= 0; i <<= 1) {
iy -= 1;
}
} else {
iy = (hy >> 23) - 127;
}

/* set up {hx,lx}, {hy,ly} and align y to x */
if (ix >= -126) {
hx = 0x00800000 | (0x007fffff & hx);
} else {
/* subnormal x, shift x to normal */
n = -126 - ix;
hx = hx << n;
}
if (iy >= -126) {
hy = 0x00800000 | (0x007fffff & hy);
} else {
/* subnormal y, shift y to normal */
n = -126 - iy;
hy = hy << n;
}

/* fix point fmod */
n = ix - iy;
while (n-- != 0) {
hz = hx - hy;
if (hz < 0) {
hx = hx + hx;
} else {
if (hz == 0) {
return signedZero(sx);
}
hx = hz + hz;
}
}
hz = hx - hy;
if (hz >= 0) {
hx = hz;
}

if (Float.isInfinite(y) || x == 0.0f) {
return x;
/* convert back to floating value and restore the sign */
if (hx == 0) {
return signedZero(sx);
}
while (hx < 0x00800000) {
/* normalize x */
hx = hx + hx;
iy -= 1;
}
if (iy >= -126) {
/* normalize output */
hx = ((hx - 0x00800000) | ((iy + 127) << 23));
x = Float.intBitsToFloat(hx | sx);
} else {
/* subnormal output */
n = -126 - iy;
hx >>= n;
x = Float.intBitsToFloat(hx | sx);
x *= 1.0f; /* create necessary signal */
}

return x; /* exact output */
}

return x - (y * narrowFloatToInt(x / y));
private static float signedZero(int sx) {
return sx == 0 ? .0f : -.0f;
}

/** Narrows a number to an unsigned 16-bit number. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package binaryexpressions;

import static com.google.j2cl.integration.testing.Asserts.assertEquals;
import static com.google.j2cl.integration.testing.Asserts.assertEqualsDelta;
import static com.google.j2cl.integration.testing.Asserts.assertTrue;

Expand Down Expand Up @@ -110,8 +111,16 @@ private static void testFloatingPointRemainder() {
assertTrue(Double.isNaN(-1.2 % 0.0));
assertTrue(1.2 % Double.POSITIVE_INFINITY == 1.2);
assertTrue(1.2 % Double.NEGATIVE_INFINITY == 1.2);
assertTrue(0.0 % 1.2 == 0.0);
assertTrue(-0.0 % 1.2 == -0.0);
assertTrue(Long.MAX_VALUE % 3.0 == 2.0);
assertTrue(Long.MAX_VALUE % -3.0 == 2.0);
assertTrue(Long.MIN_VALUE % 3.0 == -2.0);
assertTrue(Long.MIN_VALUE % -3.0 == -2.0);
assertEquals(1.0 % 1.0, 0.0);
assertEquals(-1.0 % 1.0, -0.0);
assertEquals(1.0 % -1.0, 0.0);
assertEquals(-1.0 % -1.0, -0.0);
assertEquals(0.0 % 1.2, 0.0);
assertEquals(-0.0 % 1.2, -0.0);
assertEqualsDelta(.2, .7 % .5, 1e-15);
assertEqualsDelta(.2, .7 % -.5, 1e-15);
assertEqualsDelta(-.2, -.7 % .5, 1e-15);
Expand All @@ -125,8 +134,16 @@ private static void testFloatingPointRemainder() {
assertTrue(Float.isNaN(-1.2f % 0.0f));
assertTrue(1.2f % Float.POSITIVE_INFINITY == 1.2f);
assertTrue(1.2f % Float.NEGATIVE_INFINITY == 1.2f);
assertTrue(0.0f % 1.2f == 0.0f);
assertTrue(-0.0f % 1.2f == -0.0f);
assertTrue(Long.MAX_VALUE % 3.0f == 2.0f);
assertTrue(Long.MAX_VALUE % -3.0f == 2.0f);
assertTrue(Long.MIN_VALUE % 3.0f == -2.0f);
assertTrue(Long.MIN_VALUE % -3.0f == -2.0f);
assertEquals(1.0f % 1.0f, 0.0f);
assertEquals(-1.0f % 1.0f, -0.0f);
assertEquals(1.0f % -1.0f, 0.0f);
assertEquals(-1.0f % -1.0f, -0.0f);
assertEquals(0.0f % 1.2f, 0.0f);
assertEquals(-0.0f % 1.2f, -0.0f);
assertEqualsDelta(.2f, .7f % .5f, 1e-7);
assertEqualsDelta(.2f, .7f % -.5f, 1e-7);
assertEqualsDelta(-.2f, -.7f % .5f, 1e-7);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ function getImports() {
'get RegExpResult.index': (/** !RegExpResult */ $instance, ) => $instance.index,
'get RegExpResult.length': (/** !RegExpResult */ $instance, ) => $instance.length,
'RegExpResult.at': (/** !RegExpResult */ $instance, /** number */ index, ) => $instance.at(index, ),
'j2wasm.DoubleUtils.dmod': j2wasm_DoubleUtils.dmod,
'Number.isInteger': Number.isInteger,
'RegExp.toString': (/** !RegExp */ $instance, ) => $instance.toString(),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ function getImports() {
'get RegExpResult.index': (/** !RegExpResult */ $instance, ) => $instance.index,
'get RegExpResult.length': (/** !RegExpResult */ $instance, ) => $instance.length,
'RegExpResult.at': (/** !RegExpResult */ $instance, /** number */ index, ) => $instance.at(index, ),
'j2wasm.DoubleUtils.dmod': j2wasm_DoubleUtils.dmod,
'nativejstypes.Bar.constructor': (/** number */ x, /** number */ y, ) => new nativejstypes_Bar(x, y, ),
'nativejstypes.Bar.product': (/** !nativejstypes_Bar */ $instance, ) => $instance.product(),
'nativejstypes.Bar.getStatic': nativejstypes_Bar.getStatic,
Expand Down

0 comments on commit 7cc0f3a

Please sign in to comment.