Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose float from_bits and to_bits in libcore. #46931

Merged
merged 2 commits into from
Jan 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 24 additions & 36 deletions src/libcore/num/dec2flt/rawfp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,10 @@
//! Many functions in this module only handle normal numbers. The dec2flt routines conservatively
//! take the universally-correct slow path (Algorithm M) for very small and very large numbers.
//! That algorithm needs only next_float() which does handle subnormals and zeros.
use u32;
use cmp::Ordering::{Less, Equal, Greater};
use ops::{Mul, Div, Neg};
use convert::{TryFrom, TryInto};
use ops::{Add, Mul, Div, Neg};
use fmt::{Debug, LowerExp};
use mem::transmute;
use num::diy_float::Fp;
use num::FpCategory::{Infinite, Zero, Subnormal, Normal, Nan};
use num::Float;
Expand All @@ -56,22 +55,27 @@ impl Unpacked {
///
/// Should **never ever** be implemented for other types or be used outside the dec2flt module.
/// Inherits from `Float` because there is some overlap, but all the reused methods are trivial.
pub trait RawFloat : Float + Copy + Debug + LowerExp
+ Mul<Output=Self> + Div<Output=Self> + Neg<Output=Self>
pub trait RawFloat
: Float
+ Copy
+ Debug
+ LowerExp
+ Mul<Output=Self>
+ Div<Output=Self>
+ Neg<Output=Self>
where
Self: Float<Bits = <Self as RawFloat>::RawBits>
{
const INFINITY: Self;
const NAN: Self;
const ZERO: Self;

/// Same as `Float::Bits` with extra traits.
type RawBits: Add<Output = Self::RawBits> + From<u8> + TryFrom<u64>;

/// Returns the mantissa, exponent and sign as integers.
fn integer_decode(self) -> (u64, i16, i8);

/// Get the raw binary representation of the float.
fn transmute(self) -> u64;

/// Transmute the raw binary representation into a float.
fn from_bits(bits: u64) -> Self;

/// Decode the float.
fn unpack(self) -> Unpacked;

Expand Down Expand Up @@ -149,6 +153,8 @@ macro_rules! other_constants {
}

impl RawFloat for f32 {
type RawBits = u32;

const SIG_BITS: u8 = 24;
const EXP_BITS: u8 = 8;
const CEIL_LOG5_OF_MAX_SIG: i16 = 11;
Expand All @@ -159,7 +165,7 @@ impl RawFloat for f32 {

/// Returns the mantissa, exponent and sign as integers.
fn integer_decode(self) -> (u64, i16, i8) {
let bits: u32 = unsafe { transmute(self) };
let bits = self.to_bits();
let sign: i8 = if bits >> 31 == 0 { 1 } else { -1 };
let mut exponent: i16 = ((bits >> 23) & 0xff) as i16;
let mantissa = if exponent == 0 {
Expand All @@ -172,16 +178,6 @@ impl RawFloat for f32 {
(mantissa as u64, exponent, sign)
}

fn transmute(self) -> u64 {
let bits: u32 = unsafe { transmute(self) };
bits as u64
}

fn from_bits(bits: u64) -> f32 {
assert!(bits < u32::MAX as u64, "f32::from_bits: too many bits");
unsafe { transmute(bits as u32) }
}

fn unpack(self) -> Unpacked {
let (sig, exp, _sig) = self.integer_decode();
Unpacked::new(sig, exp)
Expand All @@ -200,6 +196,8 @@ impl RawFloat for f32 {


impl RawFloat for f64 {
type RawBits = u64;

const SIG_BITS: u8 = 53;
const EXP_BITS: u8 = 11;
const CEIL_LOG5_OF_MAX_SIG: i16 = 23;
Expand All @@ -210,7 +208,7 @@ impl RawFloat for f64 {

/// Returns the mantissa, exponent and sign as integers.
fn integer_decode(self) -> (u64, i16, i8) {
let bits: u64 = unsafe { transmute(self) };
let bits = self.to_bits();
let sign: i8 = if bits >> 63 == 0 { 1 } else { -1 };
let mut exponent: i16 = ((bits >> 52) & 0x7ff) as i16;
let mantissa = if exponent == 0 {
Expand All @@ -223,15 +221,6 @@ impl RawFloat for f64 {
(mantissa, exponent, sign)
}

fn transmute(self) -> u64 {
let bits: u64 = unsafe { transmute(self) };
bits
}

fn from_bits(bits: u64) -> f64 {
unsafe { transmute(bits) }
}

fn unpack(self) -> Unpacked {
let (sig, exp, _sig) = self.integer_decode();
Unpacked::new(sig, exp)
Expand Down Expand Up @@ -296,14 +285,14 @@ pub fn encode_normal<T: RawFloat>(x: Unpacked) -> T {
"encode_normal: exponent out of range");
// Leave sign bit at 0 ("+"), our numbers are all positive
let bits = (k_enc as u64) << T::EXPLICIT_SIG_BITS | sig_enc;
T::from_bits(bits)
T::from_bits(bits.try_into().unwrap_or_else(|_| unreachable!()))
}

/// Construct a subnormal. A mantissa of 0 is allowed and constructs zero.
pub fn encode_subnormal<T: RawFloat>(significand: u64) -> T {
assert!(significand < T::MIN_SIG, "encode_subnormal: not actually subnormal");
// Encoded exponent is 0, the sign bit is 0, so we just have to reinterpret the bits.
T::from_bits(significand)
T::from_bits(significand.try_into().unwrap_or_else(|_| unreachable!()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a step back, isn't there a way to keep this using just from_bits? Do you know if it has any performance impact?

The assertion assert!(bits < u32::MAX as u64, "f32::from_bits: too many bits"); from the old code has been lost, but I suppose it is unreachable here.

Copy link
Contributor

@hanna-kruppe hanna-kruppe Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't worry about performance impact, this is only called near the end of the slowest of slow paths. Edit: The other instance of this pattern, in encode_normal above, is also called in the slightly-less-slow-path, Algorithm R, but that path is still really slow in absolute terms. So I stand by my assesment.

It's still ugly as hell but that ugliness will have to be somewhere unless all the dec2flt code is restructured to work with the Bits associated type instead of u64 (which would probably not be pretty either). Maybe it could move into RawFloat::from_bits? (But if so, it should gain a nicer panic message than unreachable)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try seeing what needs to be done to use Bits instead of u64 here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably far too big a refactor to be worthwhile for this PR.

Copy link
Contributor

@hanna-kruppe hanna-kruppe Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the try_into to from_bits seems more fruitful.

}

/// Approximate a bignum with an Fp. Rounds within 0.5 ULP with half-to-even.
Expand Down Expand Up @@ -363,8 +352,7 @@ pub fn next_float<T: RawFloat>(x: T) -> T {
// too is exactly what we want!
// Finally, f64::MAX + 1 = 7eff...f + 1 = 7ff0...0 = f64::INFINITY.
Zero | Subnormal | Normal => {
let bits: u64 = x.transmute();
T::from_bits(bits + 1)
T::from_bits(x.to_bits() + T::Bits::from(1u8))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This technically changes the overflow behaviour on release, but I don't think it should be a problem, as this function isn't expected to ever trigger the assert that was originally in from_bits here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this can't overflow. If x was the all-ones bit pattern, it'd be a NaN and we wouldn't enter this branch.

}
}
}
24 changes: 17 additions & 7 deletions src/libcore/num/f32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ pub mod consts {
reason = "stable interface is via `impl f{32,64}` in later crates",
issue = "32110")]
impl Float for f32 {
type Bits = u32;

/// Returns `true` if the number is NaN.
#[inline]
fn is_nan(self) -> bool {
Expand Down Expand Up @@ -171,7 +173,7 @@ impl Float for f32 {
const EXP_MASK: u32 = 0x7f800000;
const MAN_MASK: u32 = 0x007fffff;

let bits: u32 = unsafe { mem::transmute(self) };
let bits = self.to_bits();
match (bits & MAN_MASK, bits & EXP_MASK) {
(0, 0) => Fp::Zero,
(_, 0) => Fp::Subnormal,
Expand Down Expand Up @@ -215,12 +217,7 @@ impl Float for f32 {
fn is_sign_negative(self) -> bool {
// IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus
// applies to zeros and NaNs as well.
#[repr(C)]
union F32Bytes {
f: f32,
b: u32
}
unsafe { F32Bytes { f: self }.b & 0x8000_0000 != 0 }
self.to_bits() & 0x8000_0000 != 0
}

/// Returns the reciprocal (multiplicative inverse) of the number.
Expand Down Expand Up @@ -274,4 +271,17 @@ impl Float for f32 {
// multiplying by 1.0. Should switch to the `canonicalize` when it works.
(if self < other || other.is_nan() { self } else { other }) * 1.0
}

/// Raw transmutation to `u32`.
#[inline]
fn to_bits(self) -> u32 {
unsafe { mem::transmute(self) }
}

/// Raw transmutation from `u32`.
#[inline]
fn from_bits(v: u32) -> Self {
// It turns out the safety issues with sNaN were overblown! Hooray!
unsafe { mem::transmute(v) }
}
}
24 changes: 17 additions & 7 deletions src/libcore/num/f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ pub mod consts {
reason = "stable interface is via `impl f{32,64}` in later crates",
issue = "32110")]
impl Float for f64 {
type Bits = u64;

/// Returns `true` if the number is NaN.
#[inline]
fn is_nan(self) -> bool {
Expand Down Expand Up @@ -171,7 +173,7 @@ impl Float for f64 {
const EXP_MASK: u64 = 0x7ff0000000000000;
const MAN_MASK: u64 = 0x000fffffffffffff;

let bits: u64 = unsafe { mem::transmute(self) };
let bits = self.to_bits();
match (bits & MAN_MASK, bits & EXP_MASK) {
(0, 0) => Fp::Zero,
(_, 0) => Fp::Subnormal,
Expand Down Expand Up @@ -213,12 +215,7 @@ impl Float for f64 {
/// negative sign bit and negative infinity.
#[inline]
fn is_sign_negative(self) -> bool {
#[repr(C)]
union F64Bytes {
f: f64,
b: u64
}
unsafe { F64Bytes { f: self }.b & 0x8000_0000_0000_0000 != 0 }
self.to_bits() & 0x8000_0000_0000_0000 != 0
}

/// Returns the reciprocal (multiplicative inverse) of the number.
Expand Down Expand Up @@ -272,4 +269,17 @@ impl Float for f64 {
// multiplying by 1.0. Should switch to the `canonicalize` when it works.
(if self < other || other.is_nan() { self } else { other }) * 1.0
}

/// Raw transmutation to `u64`.
#[inline]
fn to_bits(self) -> u64 {
unsafe { mem::transmute(self) }
}

/// Raw transmutation from `u64`.
#[inline]
fn from_bits(v: u64) -> Self {
// It turns out the safety issues with sNaN were overblown! Hooray!
unsafe { mem::transmute(v) }
}
}
11 changes: 11 additions & 0 deletions src/libcore/num/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2872,6 +2872,10 @@ pub enum FpCategory {
reason = "stable interface is via `impl f{32,64}` in later crates",
issue = "32110")]
pub trait Float: Sized {
/// Type used by `to_bits` and `from_bits`.
#[stable(feature = "core_float_bits", since = "1.24.0")]
type Bits;

/// Returns `true` if this value is NaN and false otherwise.
#[stable(feature = "core", since = "1.6.0")]
fn is_nan(self) -> bool;
Expand Down Expand Up @@ -2933,6 +2937,13 @@ pub trait Float: Sized {
/// Returns the minimum of the two numbers.
#[stable(feature = "core_float_min_max", since="1.20.0")]
fn min(self, other: Self) -> Self;

/// Raw transmutation to integer.
#[stable(feature = "core_float_bits", since="1.24.0")]
fn to_bits(self) -> Self::Bits;
/// Raw transmutation from integer.
#[stable(feature = "core_float_bits", since="1.24.0")]
fn from_bits(v: Self::Bits) -> Self;
}

macro_rules! from_str_radix_int_impl {
Expand Down
5 changes: 2 additions & 3 deletions src/libstd/f32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ impl f32 {
#[stable(feature = "float_bits_conv", since = "1.20.0")]
#[inline]
pub fn to_bits(self) -> u32 {
unsafe { ::mem::transmute(self) }
num::Float::to_bits(self)
}

/// Raw transmutation from `u32`.
Expand Down Expand Up @@ -1060,8 +1060,7 @@ impl f32 {
#[stable(feature = "float_bits_conv", since = "1.20.0")]
#[inline]
pub fn from_bits(v: u32) -> Self {
// It turns out the safety issues with sNaN were overblown! Hooray!
unsafe { ::mem::transmute(v) }
num::Float::from_bits(v)
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/libstd/f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,7 @@ impl f64 {
#[stable(feature = "float_bits_conv", since = "1.20.0")]
#[inline]
pub fn to_bits(self) -> u64 {
unsafe { ::mem::transmute(self) }
num::Float::to_bits(self)
}

/// Raw transmutation from `u64`.
Expand Down Expand Up @@ -1015,8 +1015,7 @@ impl f64 {
#[stable(feature = "float_bits_conv", since = "1.20.0")]
#[inline]
pub fn from_bits(v: u64) -> Self {
// It turns out the safety issues with sNaN were overblown! Hooray!
unsafe { ::mem::transmute(v) }
num::Float::from_bits(v)
}
}

Expand Down