core::num::dec2flt interprets the rounding mode incorrectly for negative numbers

[eseraygun]

core::num::dec2flt interprets the floating-point rounding mode incorrectly for negative numbers. The assumption in the implementation that negative numbers can be handled by just flipping the sign bit is wrong in the general case.

Observed Behavior

core::num::dec2flt rounds negative numbers upwards when the rounding mode is set to downwards and vice versa.

Here is the code that demonstrates the issue.

extern crate libc;

use libc::c_int;
use std::str::FromStr;

const FE_TONEAREST: c_int = 0;
const FE_DOWNWARD:  c_int = 0x400;
const FE_UPWARD:    c_int = 0x800;

extern {
    fn fesetround(rdir: c_int) -> c_int;
}

fn main() {
    unsafe { fesetround(FE_DOWNWARD); }
    let p_lo = f64::from_str("0.9").unwrap();
    let n_lo = f64::from_str("-0.9").unwrap();

    unsafe { fesetround(FE_UPWARD); }
    let p_hi = f64::from_str("0.9").unwrap();
    let n_hi = f64::from_str("-0.9").unwrap();

    unsafe { fesetround(FE_TONEAREST); }
    println!(" 0.9: <{:19.16}, {:19.16}>", p_lo, p_hi);
    println!("-0.9: <{:19.16}, {:19.16}>", n_lo, n_hi);
}

Output:

 0.9: < 0.8999999999999999,  0.9000000000000000>
-0.9: <-0.8999999999999999, -0.9000000000000000>

Here is the equivalent code in C.

#include <fenv.h>
#include <stdio.h>
#include <stdlib.h>

int main() {
    double p_lo, p_hi, n_lo, n_hi;

    fesetround(FE_DOWNWARD);
    p_lo = strtod("0.9", NULL);
    n_lo = strtod("-0.9", NULL);

    fesetround(FE_UPWARD);
    p_hi = strtod("0.9", NULL);
    n_hi = strtod("-0.9", NULL);

    fesetround(FE_TONEAREST);
    printf(" 0.9: <%19.16f, %19.16f>\n", p_lo, p_hi);
    printf("-0.9: <%19.16f, %19.16f>\n", n_lo, n_hi);
}

Output:

 0.9: < 0.8999999999999999,  0.9000000000000000>
-0.9: <-0.9000000000000000, -0.8999999999999999>

C handles the rounding correctly whereas Rust gets it wrong for negative numbers.

Expected Behavior

core::num::dec2flt should either not be affected by the rounding mode and always use half-to-even rounding as stated in the documentation or interpret the rounding mode correctly.

Meta

rustc --version --verbose:

rustc 1.19.0-nightly (6a5fc9eec 2017-05-02)
binary: rustc
commit-hash: 6a5fc9eecec235312755e737fb5b984abe537f2e
commit-date: 2017-05-02
host: x86_64-unknown-linux-gnu
release: 1.19.0-nightly
LLVM version: 4.0

[jonas-schievink]

fesetround is likely causing undefined behaviour since LLVM doesn't support it (causing misoptimizations)

[eseraygun]

I get the same output when the optimizations are off and the strings are black_boxed.

It can still be said that the behavior of core::num::dec2flt is undefined when fesetround is used. However, that's only because of the aforementioned assumption about negative numbers. I believe a rounding mode agnostic implementation should not be much harder than the existing one: Just do subtractions instead of additions when dealing with negative numbers. I may be wrong though.

[hanna-kruppe]

Rust in general doesn't care about/support changing rounding modes at all (partly as a consequence of LLVM not supporting it well, but also because our numerical story is rather naive in general). Furthermore, even if it did in some contexts, it's far from obvious to me that parsing should respect the rounding mode (e.g., it may conflict with the goal of perfect round-tripping of float formatting). Maybe IEEE-754 implies it, I haven't checked.

I believe a rounding mode agnostic implementation should not be much harder than the existing one: Just do subtractions instead of additions when dealing with negative numbers. I may be wrong though.

I haven't thought deeply about it, but it's definitely more involved than that. This wording seems to imply you think of the conversion as the equivalent of result = 0; for each digit { result += digit * weight; }; result *= 10.pos(exponent);. This is not the case, as you'll find when you read the rest of the comments :) There are several places where rounding is performed, and most are rather involved and tailored to a specific rounding mode.

[eseraygun]

I didn't intend to imply that the conversion consists of simply summing up the weighted digits. What I tried to imply was that it is a series of standard floating operations that ultimately converges to the result. That still may be wrong of course. Also, as you rightly said, the rounding logic of the conversion may conflict with that of the CPU.

I checked the implementation of stdtod in GNU C Library and it seems to explicitly read the floating point rounding mode and perform the rounding based on its value to be able to produce the correct result. At its current state, this is too much to expect from Rust.

I would still like to see this issue resolved in the far future where Rust can handle floats as good as C does. I leave it to you whether to label this issue for future reference or just to close it.

[steveklabnik]

Triage: no changes.

[Mark-Simulacrum]

I am going to close this issue as I think we're currently not intending to utilize the floating point rounding mode within libcore (or otherwise).