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>

diff --git a/src/internal/strconv/atofeisel.go b/src/internal/strconv/atofeisel.go
index 10b8c96bba94e2f7a9d3b6e25e6eb89b6d7b7e82..5fa92908b49cedf75856a4144335e6b3b1653b3b 100644 (file)
--- a/src/internal/strconv/atofeisel.go
+++ b/src/internal/strconv/atofeisel.go
@@ -40,7 +40,7 @@ func eiselLemire64(man uint64, exp10 int, neg bool) (f float64, ok bool) {
        // Normalization.
        clz := bits.LeadingZeros64(man)
        man <<= uint(clz)
-       retExp2 := uint64(exp2+64-float64Bias) - uint64(clz)
+       retExp2 := uint64(exp2+63-float64Bias) - uint64(clz)
 
        // Multiplication.
        xHi, xLo := bits.Mul64(man, pow.Hi)
@@ -115,7 +115,7 @@ func eiselLemire32(man uint64, exp10 int, neg bool) (f float32, ok bool) {
        // Normalization.
        clz := bits.LeadingZeros64(man)
        man <<= uint(clz)
-       retExp2 := uint64(exp2+64-float32Bias) - uint64(clz)
+       retExp2 := uint64(exp2+63-float32Bias) - uint64(clz)
 
        // Multiplication.
        xHi, xLo := bits.Mul64(man, pow.Hi)

diff --git a/src/internal/strconv/export_test.go b/src/internal/strconv/export_test.go
index bea741e6fbe1bf927f347dc1ba5790253a0727e2..86435f66cf84c646da1ab76b6cd6fc263f1484a6 100644 (file)
--- a/src/internal/strconv/export_test.go
+++ b/src/internal/strconv/export_test.go
@@ -6,6 +6,11 @@ package strconv
 
 type Uint128 = uint128
 
+const (
+       Pow10Min = pow10Min
+       Pow10Max = pow10Max
+)
+
 var (
        MulLog10_2       = mulLog10_2
        MulLog2_10       = mulLog2_10

diff --git a/src/internal/strconv/ftoaryu.go b/src/internal/strconv/ftoaryu.go
index 473e5b65be8d4c6614640e9e76090732164a67b1..999af515029204041081db6fce2ab8c56514e801 100644 (file)
--- a/src/internal/strconv/ftoaryu.go
+++ b/src/internal/strconv/ftoaryu.go
@@ -464,7 +464,7 @@ func mult64bitPow10(m uint32, e2, q int) (resM uint32, resE int, exact bool) {
                pow.Hi++
        }
        hi, lo := bits.Mul64(uint64(m), pow.Hi)
-       e2 += exp2 - 63 + 57
+       e2 += exp2 - 64 + 57
        return uint32(hi<<7 | lo>>57), e2, lo<<7 == 0
 }
 
@@ -492,7 +492,7 @@ func mult128bitPow10(m uint64, e2, q int) (resM uint64, resE int, exact bool) {
                // Inverse powers of ten must be rounded up.
                pow.Lo++
        }
-       e2 += exp2 - 127 + 119
+       e2 += exp2 - 128 + 119
 
        hi, mid, lo := umul192(m, pow)
        return hi<<9 | mid>>55, e2, mid<<9 == 0 && lo == 0

diff --git a/src/internal/strconv/import_test.go b/src/internal/strconv/import_test.go
index 0cbc451651a015a0e753ca8e21d3da650ca424d1..ed1015ee5d214d12a051a20f51325eef5b5e4441 100644 (file)
--- a/src/internal/strconv/import_test.go
+++ b/src/internal/strconv/import_test.go
@@ -8,6 +8,11 @@ import . "internal/strconv"
 
 type uint128 = Uint128
 
+const (
+       pow10Min = Pow10Min
+       pow10Max = Pow10Max
+)
+
 var (
        mulLog10_2       = MulLog10_2
        mulLog2_10       = MulLog2_10

diff --git a/src/internal/strconv/math.go b/src/internal/strconv/math.go
index f0f3d5fe540b23e38350b5c5bca10133f99eabc4..37303d76dbbaf52a9ac37150f43c96da7b5c5869 100644 (file)
--- a/src/internal/strconv/math.go
+++ b/src/internal/strconv/math.go
@@ -27,13 +27,14 @@ func umul192(x uint64, y uint128) (hi, mid, lo uint64) {
        return hi + carry, mid, lo
 }
 
-// pow10 returns the 128-bit mantissa and binary exponent of 10**e
+// pow10 returns the 128-bit mantissa and binary exponent of 10**e.
+// That is, 10^e = mant/2^128 * 2**exp.
 // If e is out of range, pow10 returns ok=false.
 func pow10(e int) (mant uint128, exp int, ok bool) {
        if e < pow10Min || e > pow10Max {
                return
        }
-       return pow10Tab[e-pow10Min], mulLog2_10(e), true
+       return pow10Tab[e-pow10Min], 1 + mulLog2_10(e), true
 }
 
 // mulLog10_2 returns math.Floor(x * log(2)/log(10)) for an integer x in

diff --git a/src/internal/strconv/math_test.go b/src/internal/strconv/math_test.go
index d4f881b5e74d2060e49a700e278c4803beaec78a..3a1ff3400c06bbba982f42755ad38dd881338b85 100644 (file)
--- a/src/internal/strconv/math_test.go
+++ b/src/internal/strconv/math_test.go
@@ -5,8 +5,8 @@
 package strconv_test
 
 import (
-       "math"
        . "internal/strconv"
+       "math"
        "testing"
 )
 
@@ -17,21 +17,36 @@ var pow10Tests = []struct {
        ok    bool
 }{
        {-349, uint128{0, 0}, 0, false},
-       {-348, uint128{0xFA8FD5A0081C0288, 0x1732C869CD60E453}, -1157, true},
-       {0, uint128{0x8000000000000000, 0x0000000000000000}, 0, true},
-       {347, uint128{0xD13EB46469447567, 0x4B7195F2D2D1A9FB}, 1152, true},
+       {-348, uint128{0xFA8FD5A0081C0288, 0x1732C869CD60E453}, -1156, true},
+       {0, uint128{0x8000000000000000, 0x0000000000000000}, 1, true},
+       {347, uint128{0xD13EB46469447567, 0x4B7195F2D2D1A9FB}, 1153, true},
        {348, uint128{0, 0}, 0, false},
 }
 
 func TestPow10(t *testing.T) {
        for _, tt := range pow10Tests {
-               mant, exp2, ok := Pow10(tt.exp10)
+               mant, exp2, ok := pow10(tt.exp10)
                if mant != tt.mant || exp2 != tt.exp2 {
                        t.Errorf("pow10(%d) = %#016x, %#016x, %d, %v want %#016x,%#016x, %d, %v",
                                tt.exp10, mant.Hi, mant.Lo, exp2, ok,
                                tt.mant.Hi, tt.mant.Lo, tt.exp2, tt.ok)
                }
        }
+
+       for p := pow10Min; p <= pow10Max; p++ {
+               mant, exp2, ok := pow10(p)
+               if !ok {
+                       t.Errorf("pow10(%d) not ok", p)
+                       continue
+               }
+               // Note: -64 instead of -128 because we only used mant.Hi, not all of mant.
+               have := math.Ldexp(float64(mant.Hi), exp2-64)
+               want := math.Pow(10, float64(p))
+               if math.Abs(have-want)/want > 0.00001 {
+                       t.Errorf("pow10(%d) = %#016x%016x/2^128 * 2^%d = %g want ~%g", p, mant.Hi, mant.Lo, exp2, have, want)
+               }
+       }
+
 }
 
 func u128(hi, lo uint64) uint128 {