Skip to content

Commit c5f25b0

Browse files
aardvark179aardvark179
committed
[GR-36928] Float rounding fix
PullRequest: truffleruby/3342
2 parents b1dbba7 + 00410d1 commit c5f25b0

File tree

7 files changed

+295
-138
lines changed

7 files changed

+295
-138
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Bug fixes:
1414
* Fix `File.open` mode string parsing when binary option is the third character (@bjfish).
1515
* Fix `rb_scan_args_kw` macro to avoid shadowing variables (#2649, @aardvark179).
1616
* Fix `String#unpack("Z")` to not advance after the null byte, like CRuby (#2659, @aardvark179).
17+
* Fix `Float#round` to avoid losing precision during the rounding process (@aardvark179).
1718

1819
Compatibility:
1920

‎spec/ruby/core/float/round_spec.rb

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,70 @@
103103
5.55.round(1, half: :up).should eql(5.6)
104104
5.55.round(1, half: :down).should eql(5.5)
105105
5.55.round(1, half: :even).should eql(5.6)
106+
-5.55.round(1, half: nil).should eql(-5.6)
107+
-5.55.round(1, half: :up).should eql(-5.6)
108+
-5.55.round(1, half: :down).should eql(-5.5)
109+
-5.55.round(1, half: :even).should eql(-5.6)
110+
end
111+
112+
it "preserves cases where neighbouring floating pointer number increase the decimal places" do
113+
4.8100000000000005.round(5, half: nil).should eql(4.81)
114+
4.8100000000000005.round(5, half: :up).should eql(4.81)
115+
4.8100000000000005.round(5, half: :down).should eql(4.81)
116+
4.8100000000000005.round(5, half: :even).should eql(4.81)
117+
-4.8100000000000005.round(5, half: nil).should eql(-4.81)
118+
-4.8100000000000005.round(5, half: :up).should eql(-4.81)
119+
-4.8100000000000005.round(5, half: :down).should eql(-4.81)
120+
-4.8100000000000005.round(5, half: :even).should eql(-4.81)
121+
4.81.round(5, half: nil).should eql(4.81)
122+
4.81.round(5, half: :up).should eql(4.81)
123+
4.81.round(5, half: :down).should eql(4.81)
124+
4.81.round(5, half: :even).should eql(4.81)
125+
-4.81.round(5, half: nil).should eql(-4.81)
126+
-4.81.round(5, half: :up).should eql(-4.81)
127+
-4.81.round(5, half: :down).should eql(-4.81)
128+
-4.81.round(5, half: :even).should eql(-4.81)
129+
4.809999999999999.round(5, half: nil).should eql(4.81)
130+
4.809999999999999.round(5, half: :up).should eql(4.81)
131+
4.809999999999999.round(5, half: :down).should eql(4.81)
132+
4.809999999999999.round(5, half: :even).should eql(4.81)
133+
-4.809999999999999.round(5, half: nil).should eql(-4.81)
134+
-4.809999999999999.round(5, half: :up).should eql(-4.81)
135+
-4.809999999999999.round(5, half: :down).should eql(-4.81)
136+
-4.809999999999999.round(5, half: :even).should eql(-4.81)
137+
end
138+
139+
ruby_bug "", ""..."3.2" do
140+
# These numbers are neighbouring floating point numbers round a
141+
# precise value. They test that the rounding modes work correctly
142+
# round that value and precision is not lost which might cause
143+
# incorrect results.
144+
it "does not lose precision during the rounding process" do
145+
767573.1875850001.round(5, half: nil).should eql(767573.18759)
146+
767573.1875850001.round(5, half: :up).should eql(767573.18759)
147+
767573.1875850001.round(5, half: :down).should eql(767573.18759)
148+
767573.1875850001.round(5, half: :even).should eql(767573.18759)
149+
-767573.1875850001.round(5, half: nil).should eql(-767573.18759)
150+
-767573.1875850001.round(5, half: :up).should eql(-767573.18759)
151+
-767573.1875850001.round(5, half: :down).should eql(-767573.18759)
152+
-767573.1875850001.round(5, half: :even).should eql(-767573.18759)
153+
767573.187585.round(5, half: nil).should eql(767573.18759)
154+
767573.187585.round(5, half: :up).should eql(767573.18759)
155+
767573.187585.round(5, half: :down).should eql(767573.18758)
156+
767573.187585.round(5, half: :even).should eql(767573.18758)
157+
-767573.187585.round(5, half: nil).should eql(-767573.18759)
158+
-767573.187585.round(5, half: :up).should eql(-767573.18759)
159+
-767573.187585.round(5, half: :down).should eql(-767573.18758)
160+
-767573.187585.round(5, half: :even).should eql(-767573.18758)
161+
767573.1875849998.round(5, half: nil).should eql(767573.18758)
162+
767573.1875849998.round(5, half: :up).should eql(767573.18758)
163+
767573.1875849998.round(5, half: :down).should eql(767573.18758)
164+
767573.1875849998.round(5, half: :even).should eql(767573.18758)
165+
-767573.1875849998.round(5, half: nil).should eql(-767573.18758)
166+
-767573.1875849998.round(5, half: :up).should eql(-767573.18758)
167+
-767573.1875849998.round(5, half: :down).should eql(-767573.18758)
168+
-767573.1875849998.round(5, half: :even).should eql(-767573.18758)
169+
end
106170
end
107171

108172
it "raises FloatDomainError for exceptional values with a half option" do

‎src/main/java/org/truffleruby/core/CoreLibrary.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,6 +1026,7 @@ public boolean isTruffleBootMainMethod(SharedMethodInfo info) {
10261026
"/core/dir.rb",
10271027
"/core/dir_glob.rb",
10281028
"/core/file_test.rb",
1029+
"/core/truffle/float_operations.rb",
10291030
"/core/float.rb",
10301031
"/core/marshal.rb",
10311032
"/core/object_space.rb",

‎src/main/java/org/truffleruby/core/MathNodes.java

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -293,29 +293,43 @@ protected double doFunction(double a) {
293293
@Primitive(name = "math_frexp")
294294
public abstract static class FrExpNode extends PrimitiveArrayArgumentsNode {
295295

296+
private static final long SIGN_MASK = 1L << 63;
297+
private static final long BIASED_EXP_MASK = 0x7ffL << 52;
298+
private static final long MANTISSA_MASK = ~(SIGN_MASK | BIASED_EXP_MASK);
299+
296300
@Specialization
297301
protected RubyArray frexp(double a) {
298302
double mantissa = a;
299-
short sign = 1;
300303
long exponent = 0;
301304

302-
if (!Double.isInfinite(mantissa) && mantissa != 0.0) {
303-
// Make mantissa same sign so we only have one code path.
304-
if (mantissa < 0) {
305-
mantissa = -mantissa;
306-
sign = -1;
307-
}
308-
309-
// Increase value to hit lower range.
310-
for (; mantissa < 0.5; mantissa *= 2.0, exponent -= 1) {
311-
}
312-
313-
// Decrease value to hit upper range.
314-
for (; mantissa >= 1.0; mantissa *= 0.5, exponent += 1) {
305+
if (Double.isFinite(mantissa) && mantissa != 0.0) {
306+
307+
/* Double precision floating pointer numbers are represented as a sign, an exponent (which is biased by
308+
* 1023, and then a remaining 52 bits of mantissa. The mantissa has an implicit 53 bit which is not
309+
* stored and is always 1. There are two exceptions to this. Non-finite numbers have all the bits in
310+
* their exponent set, and if none of the exponent bits are set then the number is a signed zero, or
311+
* subnormal, and the implicit 53 bit is 0. See
312+
* https://en.wikipedia.org/wiki/Double-precision_floating-point_format for further details. */
313+
314+
final long bits = Double.doubleToRawLongBits(a);
315+
long biasedExp = ((bits & BIASED_EXP_MASK) >> 52);
316+
long mantissaBits = bits & MANTISSA_MASK;
317+
final long signBits = bits & SIGN_MASK;
318+
if (biasedExp == 0) {
319+
// Sub normal cases are a little special.
320+
// Find the most significant bit in the mantissa
321+
final int lz = Long.numberOfLeadingZeros(mantissaBits);
322+
// Shift the mantissa to make it a normal mantissa
323+
// and mask off the leading bit (now the implied 53 bit of the mantissa)..
324+
mantissaBits = (mantissaBits << (lz - 11)) & MANTISSA_MASK;
325+
// Adjust the exponent to reflect this.
326+
biasedExp = biasedExp + (lz - 11);
315327
}
328+
exponent = biasedExp - 1022;
329+
mantissa = Double.longBitsToDouble(signBits | mantissaBits | 0x3fe0000000000000L);
316330
}
317331

318-
return createArray(new Object[]{ sign * mantissa, exponent });
332+
return createArray(new Object[]{ mantissa, exponent });
319333
}
320334

321335
@Fallback

0 commit comments

Comments
 (0)