Skip to content

Commit 9795c7b

Browse files
rscgopherbot
authored andcommitted
internal/strconv: fix pow10 off-by-one in exponent result
The exact meaning of pow10 was not defined nor tested directly. Define it as pow10(e) returns mant, exp where mant/2^128 * 2**exp = 10^e. This is the most natural definition but is off-by-one from what it had been returning. Fix the off-by-one and then adjust the call sites to stop compensating for it. Change-Id: I9ee475854f30be4bd0d4f4d770a6b12ec68281fe Reviewed-on: https://go-review.googlesource.com/c/go/+/717180 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com> Auto-Submit: Russ Cox <rsc@golang.org>
1 parent ad5e941 commit 9795c7b

File tree

6 files changed

+37
-11
lines changed

6 files changed

+37
-11
lines changed

src/internal/strconv/atofeisel.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func eiselLemire64(man uint64, exp10 int, neg bool) (f float64, ok bool) {
4040
// Normalization.
4141
clz := bits.LeadingZeros64(man)
4242
man <<= uint(clz)
43-
retExp2 := uint64(exp2+64-float64Bias) - uint64(clz)
43+
retExp2 := uint64(exp2+63-float64Bias) - uint64(clz)
4444

4545
// Multiplication.
4646
xHi, xLo := bits.Mul64(man, pow.Hi)
@@ -115,7 +115,7 @@ func eiselLemire32(man uint64, exp10 int, neg bool) (f float32, ok bool) {
115115
// Normalization.
116116
clz := bits.LeadingZeros64(man)
117117
man <<= uint(clz)
118-
retExp2 := uint64(exp2+64-float32Bias) - uint64(clz)
118+
retExp2 := uint64(exp2+63-float32Bias) - uint64(clz)
119119

120120
// Multiplication.
121121
xHi, xLo := bits.Mul64(man, pow.Hi)

src/internal/strconv/export_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ package strconv
66

77
type Uint128 = uint128
88

9+
const (
10+
Pow10Min = pow10Min
11+
Pow10Max = pow10Max
12+
)
13+
914
var (
1015
MulLog10_2 = mulLog10_2
1116
MulLog2_10 = mulLog2_10

src/internal/strconv/ftoaryu.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ func mult64bitPow10(m uint32, e2, q int) (resM uint32, resE int, exact bool) {
464464
pow.Hi++
465465
}
466466
hi, lo := bits.Mul64(uint64(m), pow.Hi)
467-
e2 += exp2 - 63 + 57
467+
e2 += exp2 - 64 + 57
468468
return uint32(hi<<7 | lo>>57), e2, lo<<7 == 0
469469
}
470470

@@ -492,7 +492,7 @@ func mult128bitPow10(m uint64, e2, q int) (resM uint64, resE int, exact bool) {
492492
// Inverse powers of ten must be rounded up.
493493
pow.Lo++
494494
}
495-
e2 += exp2 - 127 + 119
495+
e2 += exp2 - 128 + 119
496496

497497
hi, mid, lo := umul192(m, pow)
498498
return hi<<9 | mid>>55, e2, mid<<9 == 0 && lo == 0

src/internal/strconv/import_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ import . "internal/strconv"
88

99
type uint128 = Uint128
1010

11+
const (
12+
pow10Min = Pow10Min
13+
pow10Max = Pow10Max
14+
)
15+
1116
var (
1217
mulLog10_2 = MulLog10_2
1318
mulLog2_10 = MulLog2_10

src/internal/strconv/math.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@ func umul192(x uint64, y uint128) (hi, mid, lo uint64) {
2727
return hi + carry, mid, lo
2828
}
2929

30-
// pow10 returns the 128-bit mantissa and binary exponent of 10**e
30+
// pow10 returns the 128-bit mantissa and binary exponent of 10**e.
31+
// That is, 10^e = mant/2^128 * 2**exp.
3132
// If e is out of range, pow10 returns ok=false.
3233
func pow10(e int) (mant uint128, exp int, ok bool) {
3334
if e < pow10Min || e > pow10Max {
3435
return
3536
}
36-
return pow10Tab[e-pow10Min], mulLog2_10(e), true
37+
return pow10Tab[e-pow10Min], 1 + mulLog2_10(e), true
3738
}
3839

3940
// mulLog10_2 returns math.Floor(x * log(2)/log(10)) for an integer x in

src/internal/strconv/math_test.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
package strconv_test
66

77
import (
8-
"math"
98
. "internal/strconv"
9+
"math"
1010
"testing"
1111
)
1212

@@ -17,21 +17,36 @@ var pow10Tests = []struct {
1717
ok bool
1818
}{
1919
{-349, uint128{0, 0}, 0, false},
20-
{-348, uint128{0xFA8FD5A0081C0288, 0x1732C869CD60E453}, -1157, true},
21-
{0, uint128{0x8000000000000000, 0x0000000000000000}, 0, true},
22-
{347, uint128{0xD13EB46469447567, 0x4B7195F2D2D1A9FB}, 1152, true},
20+
{-348, uint128{0xFA8FD5A0081C0288, 0x1732C869CD60E453}, -1156, true},
21+
{0, uint128{0x8000000000000000, 0x0000000000000000}, 1, true},
22+
{347, uint128{0xD13EB46469447567, 0x4B7195F2D2D1A9FB}, 1153, true},
2323
{348, uint128{0, 0}, 0, false},
2424
}
2525

2626
func TestPow10(t *testing.T) {
2727
for _, tt := range pow10Tests {
28-
mant, exp2, ok := Pow10(tt.exp10)
28+
mant, exp2, ok := pow10(tt.exp10)
2929
if mant != tt.mant || exp2 != tt.exp2 {
3030
t.Errorf("pow10(%d) = %#016x, %#016x, %d, %v want %#016x,%#016x, %d, %v",
3131
tt.exp10, mant.Hi, mant.Lo, exp2, ok,
3232
tt.mant.Hi, tt.mant.Lo, tt.exp2, tt.ok)
3333
}
3434
}
35+
36+
for p := pow10Min; p <= pow10Max; p++ {
37+
mant, exp2, ok := pow10(p)
38+
if !ok {
39+
t.Errorf("pow10(%d) not ok", p)
40+
continue
41+
}
42+
// Note: -64 instead of -128 because we only used mant.Hi, not all of mant.
43+
have := math.Ldexp(float64(mant.Hi), exp2-64)
44+
want := math.Pow(10, float64(p))
45+
if math.Abs(have-want)/want > 0.00001 {
46+
t.Errorf("pow10(%d) = %#016x%016x/2^128 * 2^%d = %g want ~%g", p, mant.Hi, mant.Lo, exp2, have, want)
47+
}
48+
}
49+
3550
}
3651

3752
func u128(hi, lo uint64) uint128 {

0 commit comments

Comments
 (0)