Skip to content

Commit

Permalink
reflect: when Converting between float32s, don't lose signal NaNs
Browse files Browse the repository at this point in the history
Trying this CL again, with a test that skips 387.

When converting from float32->float64->float32, any signal NaNs
get converted to quiet NaNs. Avoid that so using reflect.Value.Convert
between two float32 types keeps the signal bit of NaNs.

Skip the test on 387. I don't see any sane way of ensuring that a
float load + float store is faithful on that platform.

Fixes #36400

Change-Id: Ic316c74ddc155632e40424e207375b5d50dcd853
Reviewed-on: https://go-review.googlesource.com/c/go/+/221792
Run-TryBot: Keith Randall <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Josh Bleecher Snyder <[email protected]>
  • Loading branch information
randall77 committed Apr 1, 2020
1 parent 8e6a8d9 commit 7ffbea9
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
31 changes: 31 additions & 0 deletions src/reflect/all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4163,6 +4163,37 @@ func TestConvert(t *testing.T) {
}
}

var gFloat32 float32

func TestConvertNaNs(t *testing.T) {
const snan uint32 = 0x7f800001

// Test to see if a store followed by a load of a signaling NaN
// maintains the signaling bit. The only platform known to fail
// this test is 386,GO386=387. The real test below will always fail
// if the platform can't even store+load a float without mucking
// with the bits.
gFloat32 = math.Float32frombits(snan)
runtime.Gosched() // make sure we don't optimize the store/load away
r := math.Float32bits(gFloat32)
if r != snan {
// This should only happen on 386,GO386=387. We have no way to
// test for 387, so we just make sure we're at least on 386.
if runtime.GOARCH != "386" {
t.Errorf("store/load of sNaN not faithful")
}
t.Skip("skipping test, float store+load not faithful")
}

type myFloat32 float32
x := V(myFloat32(math.Float32frombits(snan)))
y := x.Convert(TypeOf(float32(0)))
z := y.Interface().(float32)
if got := math.Float32bits(z); got != snan {
t.Errorf("signaling nan conversion got %x, want %x", got, snan)
}
}

type ComparableStruct struct {
X int
}
Expand Down
14 changes: 14 additions & 0 deletions src/reflect/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -2541,6 +2541,14 @@ func makeFloat(f flag, v float64, t Type) Value {
return Value{typ, ptr, f | flagIndir | flag(typ.Kind())}
}

// makeFloat returns a Value of type t equal to v, where t is a float32 type.
func makeFloat32(f flag, v float32, t Type) Value {
typ := t.common()
ptr := unsafe_New(typ)
*(*float32)(ptr) = v
return Value{typ, ptr, f | flagIndir | flag(typ.Kind())}
}

// makeComplex returns a Value of type t equal to v (possibly truncated to complex64),
// where t is a complex64 or complex128 type.
func makeComplex(f flag, v complex128, t Type) Value {
Expand Down Expand Up @@ -2613,6 +2621,12 @@ func cvtUintFloat(v Value, t Type) Value {

// convertOp: floatXX -> floatXX
func cvtFloat(v Value, t Type) Value {
if v.Type().Kind() == Float32 && t.Kind() == Float32 {
// Don't do any conversion if both types have underlying type float32.
// This avoids converting to float64 and back, which will
// convert a signaling NaN to a quiet NaN. See issue 36400.
return makeFloat32(v.flag.ro(), *(*float32)(v.ptr), t)
}
return makeFloat(v.flag.ro(), v.Float(), t)
}

Expand Down

0 comments on commit 7ffbea9

Please sign in to comment.