Re: Withdraw: Review: 4396272 - Parsing doubles fails to follow IEEE for largest decimal that should yield 0
Hello Dima, Yes that would be helpful and appreciated. There is another patch to the same files that I will be looking into, but the changes are disjoint so there should not be a significant merge issue. Thanks, Brian On Feb 21, 2013, at 7:27 PM, Dmitry Nadezhin wrote: Do you want that I prepare this FloatingDecimal/FormattedFloatingDecimal patch ? -Dima On Fri, Feb 22, 2013 at 2:32 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: I am withdrawing this patch for the time being as properly the changes should also go into the fork sun.misc.FormattedFloatingDecimal. I'll post an updated patch once it is available. Thanks, Brian On Feb 14, 2013, at 5:23 PM, Brian Burkhalter wrote: The patch below is as submitted to OpenJDK bugzilla but with enhanced comments. It pertains to the correction loop in the doubleValue() method of FloatingDecimal. The situation appears to arise when the candidate value is less than 2*Double.MIN_NORMAL as for such values the ULP is less than 2*Double.MIN_VALUE so that the intermediate result 0.5*ULP is less than Double.MIN_VALUE which rounds to zero per FP-strict and the correction is therefore zero. Thus the candidate value is unchanged. The fix is to add the ULP to twice the candidate value, obtain the intermediate result, and then halve it to obtain the adjusted candidate. I am relatively new to IEEE-754 and this area of the code so any comments would be appreciated. Thanks, Brian diff -r 1405ad6afb1e -r 36482ed9bb7e src/share/classes/sun/misc/FloatingDecimal.java --- a/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 11:09:07 2013 -0800 +++ b/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 16:47:53 2013 -0800
Re: Withdraw: Review: 4396272 - Parsing doubles fails to follow IEEE for largest decimal that should yield 0
Brian, Class FloatingDecimal contains both conversion from String to float/double and conversion from float/double to String. My change is in conversion from String to float/double. The methods if FloatingDecimal that implement this conversion are: static FloatingDecimal readJavaFormatString( String in ); double doubleValue(); float floatValue(); My change is in method doubleValue(). Class FormattedFloatingDecimal was forked from FloatingDecimal some time ago. This class is used only for conversion from String to float/double. The unused method readJavaFormatString( String in) was deleted from FormattedFloatingDecimal. Methods doubleValue() and floatValued() were not deleted, but they are never used in JDK code. So I think that the required change in FormattedFloatingDecimal is to delete methods doubleValue(), floatValue() and other unused methods and fields. Am I right ? -Dima On Fri, Feb 22, 2013 at 8:46 PM, Brian Burkhalter brian.burkhal...@oracle.com wrote: Hello Dima, Yes that would be helpful and appreciated. There is another patch to the same files that I will be looking into, but the changes are disjoint so there should not be a significant merge issue. Thanks, Brian On Feb 21, 2013, at 7:27 PM, Dmitry Nadezhin wrote: Do you want that I prepare this FloatingDecimal/FormattedFloatingDecimal patch ? -Dima On Fri, Feb 22, 2013 at 2:32 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: I am withdrawing this patch for the time being as properly the changes should also go into the fork sun.misc.FormattedFloatingDecimal. I'll post an updated patch once it is available. Thanks, Brian On Feb 14, 2013, at 5:23 PM, Brian Burkhalter wrote: The patch below is as submitted to OpenJDK bugzilla but with enhanced comments. It pertains to the correction loop in the doubleValue() method of FloatingDecimal. The situation appears to arise when the candidate value is less than 2*Double.MIN_NORMAL as for such values the ULP is less than 2*Double.MIN_VALUE so that the intermediate result 0.5*ULP is less than Double.MIN_VALUE which rounds to zero per FP-strict and the correction is therefore zero. Thus the candidate value is unchanged. The fix is to add the ULP to twice the candidate value, obtain the intermediate result, and then halve it to obtain the adjusted candidate. I am relatively new to IEEE-754 and this area of the code so any comments would be appreciated. Thanks, Brian diff -r 1405ad6afb1e -r 36482ed9bb7e src/share/classes/sun/misc/FloatingDecimal.java --- a/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 11:09:07 2013 -0800 +++ b/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 16:47:53 2013 -0800
Re: Withdraw: Review: 4396272 - Parsing doubles fails to follow IEEE for largest decimal that should yield 0
Dima, If the methods are definitely unused that would be correct. I suppose if a clean build of the JDK does not complain then it is acceptable and correct. Thanks, Brian On Feb 22, 2013, at 9:41 AM, Dmitry Nadezhin wrote: So I think that the required change in FormattedFloatingDecimal is to delete methods doubleValue(), floatValue() and other unused methods and fields. Am I right ?
Re: Withdraw: Review: 4396272 - Parsing doubles fails to follow IEEE for largest decimal that should yield 0
Dima, Great! Thanks. I will take a look later and re-post the review. Brian On Feb 22, 2013, at 12:29 PM, Dmitry Nadezhin wrote: Brian, I removed unused methods and fields from FormattedFloatingDecimal. JDK build passes. The result of hg diff is attached. -Dima On Fri, Feb 22, 2013 at 9:49 PM, Brian Burkhalter brian.burkhal...@oracle.com wrote: Dima, If the methods are definitely unused that would be correct. I suppose if a clean build of the JDK does not complain then it is acceptable and correct. Thanks, Brian On Feb 22, 2013, at 9:41 AM, Dmitry Nadezhin wrote: So I think that the required change in FormattedFloatingDecimal is to delete methods doubleValue(), floatValue() and other unused methods and fields. Am I right ? hgdiff.txt
Withdraw: Review: 4396272 - Parsing doubles fails to follow IEEE for largest decimal that should yield 0
I am withdrawing this patch for the time being as properly the changes should also go into the fork sun.misc.FormattedFloatingDecimal. I'll post an updated patch once it is available. Thanks, Brian On Feb 14, 2013, at 5:23 PM, Brian Burkhalter wrote: The patch below is as submitted to OpenJDK bugzilla but with enhanced comments. It pertains to the correction loop in the doubleValue() method of FloatingDecimal. The situation appears to arise when the candidate value is less than 2*Double.MIN_NORMAL as for such values the ULP is less than 2*Double.MIN_VALUE so that the intermediate result 0.5*ULP is less than Double.MIN_VALUE which rounds to zero per FP-strict and the correction is therefore zero. Thus the candidate value is unchanged. The fix is to add the ULP to twice the candidate value, obtain the intermediate result, and then halve it to obtain the adjusted candidate. I am relatively new to IEEE-754 and this area of the code so any comments would be appreciated. Thanks, Brian diff -r 1405ad6afb1e -r 36482ed9bb7e src/share/classes/sun/misc/FloatingDecimal.java --- a/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 11:09:07 2013 -0800 +++ b/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 16:47:53 2013 -0800
Re: Withdraw: Review: 4396272 - Parsing doubles fails to follow IEEE for largest decimal that should yield 0
Do you want that I prepare this FloatingDecimal/FormattedFloatingDecimal patch ? -Dima On Fri, Feb 22, 2013 at 2:32 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: I am withdrawing this patch for the time being as properly the changes should also go into the fork sun.misc.FormattedFloatingDecimal. I'll post an updated patch once it is available. Thanks, Brian On Feb 14, 2013, at 5:23 PM, Brian Burkhalter wrote: The patch below is as submitted to OpenJDK bugzilla but with enhanced comments. It pertains to the correction loop in the doubleValue() method of FloatingDecimal. The situation appears to arise when the candidate value is less than 2*Double.MIN_NORMAL as for such values the ULP is less than 2*Double.MIN_VALUE so that the intermediate result 0.5*ULP is less than Double.MIN_VALUE which rounds to zero per FP-strict and the correction is therefore zero. Thus the candidate value is unchanged. The fix is to add the ULP to twice the candidate value, obtain the intermediate result, and then halve it to obtain the adjusted candidate. I am relatively new to IEEE-754 and this area of the code so any comments would be appreciated. Thanks, Brian diff -r 1405ad6afb1e -r 36482ed9bb7e src/share/classes/sun/misc/FloatingDecimal.java --- a/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 11:09:07 2013 -0800 +++ b/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 16:47:53 2013 -0800
Review: 4396272 - Parsing doubles fails to follow IEEE for largest decimal that should yield 0
The patch below is as submitted to OpenJDK bugzilla but with enhanced comments.
It pertains to the correction loop in the doubleValue() method of
FloatingDecimal. The situation appears to arise when the candidate value is
less than 2*Double.MIN_NORMAL as for such values the ULP is less than
2*Double.MIN_VALUE so that the intermediate result 0.5*ULP is less than
Double.MIN_VALUE which rounds to zero per FP-strict and the correction is
therefore zero. Thus the candidate value is unchanged. The fix is to add the
ULP to twice the candidate value, obtain the intermediate result, and then
halve it to obtain the adjusted candidate.
I am relatively new to IEEE-754 and this area of the code so any comments would
be appreciated.
Thanks,
Brian
diff -r 1405ad6afb1e -r 36482ed9bb7e
src/share/classes/sun/misc/FloatingDecimal.java
--- a/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 11:09:07
2013 -0800
+++ b/src/share/classes/sun/misc/FloatingDecimal.java Thu Feb 14 16:47:53
2013 -0800
@@ -35,8 +35,16 @@
int decExponent;
chardigits[];
int nDigits;
+
+// = ((doubleAsLongBits expMask) expShift) - expBias + 1 -
bigIntNBits
+// Set by doubleToBigInt().
int bigIntExp;
+
+// Number of bits from the high order 1 bit to the low order 1 bit,
+// inclusive, of the fractional (significand) part of a double.
+// Set by doubleToBigInt().
int bigIntNBits;
+
boolean mustSetRoundDir = false;
boolean fromHex = false;
int roundDir = 0; // set by doubleValue
@@ -1604,7 +1612,50 @@
} else if ( cmpResult == 0 ){
// difference is exactly half an ULP
// round to some other value maybe, then finish
-dValue += 0.5*ulp( dValue, overvalue );
+// Fix of bug 4396272. This method has strictfp modifier
now.
+// Nevertheless, the two comments below explain why it can
work without strictfp too.
+//
+// In the logical expression tested on the if-block,
+// bigIntExp = expBiased - expBias + 1 - bigIntNBits so
that
+// bigIntExp+bigIntNBits = expBiased - expBias + 1 so the
inequality tested is
+// expBiased - expBias + 1 -expBias+2 or expBiased 1
which implies
+// exUnbiased 1 - expBias = -1022 or expUnbiased -1021
so that
+// old dValue 2^(-1021) = 2*2^(-1022) =
2*Double.MIN_NORMAL.
+// Either or = could be used in the logical
expression
+// as both branches evaluate to the same result when
equality obtains.
+//
+// As dValue becomes smaller, the ULP descreases reaching
+// Double.MIN_VALUE for dValue == 2*Double.MIN_NORMAL.
Below
+// this the intermediate result 0.5*ULP will be rounded to
+// zero per FP-strict. As a result no correction to dValue
+// will occur. To prevent this, 2*dValue + ULP is first
+// computed so that the effect of adding half the ULP is
not
+// lost, then this intermediate result is halved.
+//
+if ( bigIntExp+bigIntNBits -expBias+2 ) {
+// Here old dValue 2*Double.MIN_NORMAL
+//
+// Non-FP-strict case:
+// If overvalue == false then ulp(dValue,false) =
2*Double.MIN_VALUE and 0.5*ulp is exact.
+// If overvalue == true and dValue
2*Double.MIN_NORMAL then ulp(dValue,true) = -2*Double.MIN_VALUE and 0.5*ulp is
exact.
+// If overvalue == true and dValue ==
2*Double.MIN_NORMAL then ulp(dValue,true) == -Double.MIN_VALUE.
+// Hence 0.5*ulp is rounded to 0 in double value set
and 0.5*ulp is exact in double-extended-exponent value set.
+// In both value sets new dValue is rounded to
2*Double.MIN_NOPMAL as expected
+dValue += 0.5*ulp( dValue, overvalue );
+} else {
+// Here old dValue = 2*Double.MIN_NORMAL
+//
+// Non-FP-strict case:
+// If overvalue == false then ulp(dValue,false) ==
Double.MIN_VALUE
+// If dValue = Double.MIN_NORMAL then dValue*2 +
ulp rounds to nearest even and 0.5*(...) is exact
+// If dValue Double.MIN_NORMAL then dValue*2 + ulp
is exact and 0.5*(...) rounds to nearest even in double value set
+// and is exact in double-extended-exponent value
set and then rounds to nearest even double before storing to new dValue.
+//
