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>

6 files changed, 37 insertions, 11 deletions

diff --git a/src/internal/strconv/atofeisel.go b/src/internal/strconv/atofeisel.go
index 10b8c96bba..5fa92908b4 100644
--- 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 bea741e6fb..86435f66cf 100644
--- 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 473e5b65be..999af51502 100644
--- 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 0cbc451651..ed1015ee5d 100644
--- 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 f0f3d5fe54..37303d76db 100644
--- 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 d4f881b5e7..3a1ff3400c 100644
--- 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 {