APFloat: Rewrite It In Rust and use it for deterministic floating-point CTFE. #43554

eddyb · 2017-07-30T07:27:31Z

As part of the CTFE initiative, we're forced to find a solution for floating-point operations.
By design, IEEE-754 does not explicitly define everything in a deterministic manner, and there is some variability between platforms, at the very least (e.g. NaN payloads).

If types are to evaluate constant expressions involving type (or in the future, const) generics, that evaluation needs to be fully deterministic, even across rustc host platforms.
That is, if [T; T::X] was used in a cross-compiled library, and the evaluation of T::X executed a floating-point operation, that operation has to be reproducible on any other host, only knowing T and the definition of the X associated const (as either AST or HIR).

Failure to uphold those rules allows an associated type (e.g. <Foo as Iterator>::Item) to be seen as two (or more) different types, depending on the current host, and such type safety violations typically allow writing of a transmute in safe code, given enough generics.

The options considered by @rust-lang/compiler were:

Ban floating-point operations in generic const-evaluation contexts
Emulate floating-point operations in an uniformly deterministic fashion

The former option may seem appealing at first, but floating-point operations are allowed today, so they can't be banned wholesale, a distinction has to be made between the code that already works, and future generic contexts. Moreover, every computation that succeeded has to be cached, otherwise the generic case can be reproduced without any generics. IMO there are too many ways it can go wrong, and a single violation can be enough for an unsoundness hole.
Not to mention we may end up really wanting floating-point operations anyway, in CTFE.

I went with the latter option, and seeing how LLVM already has a library for this exact purpose (as it needs to perform optimizations independently of host floating-point capabilities), i.e. APFloat, that was what I ended up basing this PR on.
But having been burned by the low reusability of bindings that link to LLVM, and because I would rather the floating-point operations to be wrong than not deterministic or not memory-safe (APFloat does far more pointer juggling than I'm comfortable with), I decided to RIIR.

This way, we have a guarantee of no unsafe code, a bit more control over the where native floating-point might accidentally be involved, and non-LLVM backends can share it.
I've also ported all the testcases over, before any functionality, to catch any mistakes.

Currently the PR replaces all CTFE operations to go through apfloat::ieee::{Single,Double}, keeping only the bits of the f32 / f64 memory representation in between operations.
Converting from a string also double-checks that core::num and apfloat agree on the interpretation of a floating-point number literal, in case either of them has any bugs left around.

r? @nikomatsakis
f? @nagisa @est31

Huge thanks to @edef1c for first demoing usable APFloat bindings and to @chandlerc for fielding my questions on IRC about APFloat peculiarities (also upstreaming some bugfixes).

eddyb · 2017-07-30T07:36:38Z

cc @solson @oli-obk @RalfJung Any thoughts on miri switching fully to this? The API isn't that ergonomic (if you always have to go from/to bits) but we could figure something out.

eddyb · 2017-07-30T07:38:36Z

Started a crater run (although cargobomb would be more useful).

Mark-Simulacrum · 2017-07-30T14:10:00Z

If you can update the PR to fix the test case and run try for bors that'll help get the crater run started.

[01:00:47] 	thread 'apfloat::tests::to_string' panicked at 'NYI fmt_custom', /checkout/src/librustc_const_math/apfloat.rs:1680:8

eddyb · 2017-07-30T15:00:32Z

@Mark-Simulacrum You mean cargobomb? I already have crater running, it doesn't run tests.

EDIT: Crater report shows no regressions.

Mark-Simulacrum · 2017-07-30T15:42:02Z

Yeah, I meant cargobomb.

alexcrichton · 2017-07-30T18:00:38Z

It looks like 8kloc of the 11kloc file is unit tests (awesome!), in which case maybe that'd be good for a src/librustc_const_eval/tests/foo.rs test? That way if you edit tests you won't have to rebootstrap the compiler!

AFAIK all you need to do is copy the info to tests/foo.rs and everything will "just work" b/c it's a "standard" cargo layout.

eddyb · 2017-07-30T18:02:17Z

@alexcrichton Ahh the advanced technology of cargo test! That'll come in real handy.

eddyb · 2017-07-30T18:45:20Z

src/librustc_trans/mir/constant.rs

                };
-                C_floating_f64(v_f64, llty)
+                consts::bitcast(Const::from_constint(ccx, &bits).llval, llty)


@alexcrichton Was there a concern at some point that passing signaling NaNs to LLVM could trigger an exception on the host? Mostly wondering in terms of marking it as a solved problem.

Hm I don't recall any particular concerns about signaling NaNs and CTFE on the host, no. I'd think it's roughly ok to basically gloss over them though.

RalfJung · 2017-07-30T18:54:17Z

Any thoughts on miri switching fully to this? The API isn't that ergonomic (if you always have to go from/to bits) but we could figure something out.

I don't know much about floating point, and have no strong opinion on how to handle them. Rust-only softfloat sounds very reasonable.

One thing I am wodnering about this PR in general -- why is APFloat.rs not a separate crate on crates.io? Compilers may not be the only ones wanting deterministic floating point.

eddyb · 2017-07-30T20:53:49Z

@RalfJung Having started as part of an optimizing compiler, it's geared towards emulation of IEEE and PPC double-double as well as possible (not everything is complete, especially the PPC side).
Other applications might want a truly arbitrary-precision library instead.
I'm not against putting this on crates.io but I'm not sure it's wise to do it right away. I will happily place it in an rustc_apfloat crate of its own if that is desired.

eddyb · 2017-07-30T22:26:07Z

@bors try

bors · 2017-07-30T22:26:17Z

⌛ Trying commit 08d9e09 with merge 21ee79b...

@nikomatsakis

APFloat: Rewrite It In Rust and use it for deterministic floating-point CTFE. As part of the CTFE initiative, we're forced to find a solution for floating-point operations. By design, IEEE-754 does not explicitly define everything in a deterministic manner, and there is some variability between platforms, at the very least (e.g. NaN payloads). If types are to evaluate constant expressions involving type (or in the future, const) generics, that evaluation needs to be *fully deterministic*, even across `rustc` host platforms. That is, if `[T; T::X]` was used in a cross-compiled library, and the evaluation of `T::X` executed a floating-point operation, that operation has to be reproducible on *any other host*, only knowing `T` and the definition of the `X` associated const (as either AST or HIR). Failure to uphold those rules allows an associated type (e.g. `<Foo as Iterator>::Item`) to be seen as two (or more) different types, depending on the current host, and such type safety violations typically allow writing of a `transmute` in safe code, given enough generics. The options considered by @rust-lang/compiler were: 1. Ban floating-point operations in generic const-evaluation contexts 2. Emulate floating-point operations in an uniformly deterministic fashion The former option may seem appealing at first, but floating-point operations *are allowed today*, so they can't be banned wholesale, a distinction has to be made between the code that already works, and future generic contexts. *Moreover*, every computation that succeeded *has to be cached*, otherwise the generic case can be reproduced without any generics. IMO there are too many ways it can go wrong, and a single violation can be enough for an unsoundness hole. Not to mention we may end up really wanting floating-point operations *anyway*, in CTFE. I went with the latter option, and seeing how LLVM *already* has a library for this exact purpose (as it needs to perform optimizations independently of host floating-point capabilities), i.e. `APFloat`, that was what I ended up basing this PR on. But having been burned by the low reusability of bindings that link to LLVM, and because I would *rather* the floating-point operations to be wrong than not deterministic or not memory-safe (`APFloat` does far more pointer juggling than I'm comfortable with), I decided to RIIR. This way, we have a guarantee of *no* `unsafe` code, a bit more control over the where native floating-point might accidentally be involved, and non-LLVM backends can share it. I've also ported all the testcases over, *before* any functionality, to catch any mistakes. Currently the PR replaces all CTFE operations to go through `apfloat::{IeeeSingle, IeeeDouble}`, keeping only the bits of the `f32` / `f64` memory representation in between operations. Converting from a string also double-checks that `core::num` and `apfloat` agree on the interpretation of a floating-point number literal, in case either of them has any bugs left around. r? @nikomatsakis f? @nagisa @est31 <hr/> Huge thanks to @edef1c for first demoing usable `APFloat` bindings and to @chandlerc for fielding my questions on IRC about `APFloat` peculiarities (also upstreaming some bugfixes). **TODO**: * should tests live in a separate file? * maybe get more tests from some IEEE conformance testsuite * maybe go through the trouble of making a new crate * maybe describe the porting process and the decisions made (towards a rusty bias) * clean up the API a bit, the method names could be more uniform (e.g. `rounding_add`) and `OpStatus` could be more monadic (a struct instead of `(T, OpStatus)`) * add licensing information (moco lawyers checked off on this a while back FWIW)

bors · 2017-07-30T23:57:22Z

☀️ Test successful - status-travis
State: approved= try=True

Mark-Simulacrum · 2017-07-31T02:00:44Z

@brson @tomprince @frewsxcv Can one of you schedule a cargobomb run? Try build completed.

nikomatsakis

r=me if tests are separated out into their own file and cargobomb is happy.

This looks awesome. I admit I didn't really read the apfloat source, I'm trusting your tests. Skimming the compiler integration though it looks pretty smooth.

eddyb · 2017-08-02T01:04:15Z

I've split the code into modules (plus two separate unit test files) and placed it in rustc_apfloat.
Are there licensing concerns? That is, given we already ship LLVM's license, do I need to, say, have a custom license header at the top of all the files in rustc_apfloat?
Maybe replace Copyright 2017 The Rust Project Developers with something else?
cc @aturon @brson

eddyb · 2017-08-02T01:10:26Z

@bors try

If you haven't started a cargobomb run yet, please wait for this try to finish and use it instead.

aturon · 2017-08-02T14:56:12Z

@eddyb We're in the process of removing the copyright headers entirely. The section in the COPYRIGHT file is all that's needed.

Also: congrats on this huge accomplishment!

tomprince · 2017-08-05T02:09:40Z

Cargobomb results

eddyb · 2017-08-05T06:02:07Z

Thanks @tomprince! Currently waiting for all the logs to get uploaded.
Most of the failures I've seen so far are the same clippy compile error, and the rest are spurious (most of them are tests which have been written broken e.g. unsafe data races).

EDIT: all reports are uploaded, no true regression observed, proceeding...

eddyb · 2017-08-05T08:12:57Z

@bors r=nikomatsakis

bors · 2017-08-05T08:12:58Z

📌 Commit c457b26 has been approved by nikomatsakis

bors · 2017-08-05T09:51:51Z

⌛ Testing commit c457b26 with merge 1ee04eadff357267f85c9af17d5935c1e5ea8ad7...

bors · 2017-08-05T12:15:37Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 1ee04eadff357267f85c9af17d5935c1e5ea8ad7 to master...

bors · 2017-08-05T12:15:39Z

👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward

eddyb · 2017-08-05T13:12:54Z

cc @rust-lang/infra about the strange push failure above.

@bors retry

bors · 2017-08-05T13:13:03Z

⌛ Testing commit c457b26 with merge 2b82b7e...

@nikomatsakis

APFloat: Rewrite It In Rust and use it for deterministic floating-point CTFE. As part of the CTFE initiative, we're forced to find a solution for floating-point operations. By design, IEEE-754 does not explicitly define everything in a deterministic manner, and there is some variability between platforms, at the very least (e.g. NaN payloads). If types are to evaluate constant expressions involving type (or in the future, const) generics, that evaluation needs to be *fully deterministic*, even across `rustc` host platforms. That is, if `[T; T::X]` was used in a cross-compiled library, and the evaluation of `T::X` executed a floating-point operation, that operation has to be reproducible on *any other host*, only knowing `T` and the definition of the `X` associated const (as either AST or HIR). Failure to uphold those rules allows an associated type (e.g. `<Foo as Iterator>::Item`) to be seen as two (or more) different types, depending on the current host, and such type safety violations typically allow writing of a `transmute` in safe code, given enough generics. The options considered by @rust-lang/compiler were: 1. Ban floating-point operations in generic const-evaluation contexts 2. Emulate floating-point operations in an uniformly deterministic fashion The former option may seem appealing at first, but floating-point operations *are allowed today*, so they can't be banned wholesale, a distinction has to be made between the code that already works, and future generic contexts. *Moreover*, every computation that succeeded *has to be cached*, otherwise the generic case can be reproduced without any generics. IMO there are too many ways it can go wrong, and a single violation can be enough for an unsoundness hole. Not to mention we may end up really wanting floating-point operations *anyway*, in CTFE. I went with the latter option, and seeing how LLVM *already* has a library for this exact purpose (as it needs to perform optimizations independently of host floating-point capabilities), i.e. `APFloat`, that was what I ended up basing this PR on. But having been burned by the low reusability of bindings that link to LLVM, and because I would *rather* the floating-point operations to be wrong than not deterministic or not memory-safe (`APFloat` does far more pointer juggling than I'm comfortable with), I decided to RIIR. This way, we have a guarantee of *no* `unsafe` code, a bit more control over the where native floating-point might accidentally be involved, and non-LLVM backends can share it. I've also ported all the testcases over, *before* any functionality, to catch any mistakes. Currently the PR replaces all CTFE operations to go through `apfloat::ieee::{Single,Double}`, keeping only the bits of the `f32` / `f64` memory representation in between operations. Converting from a string also double-checks that `core::num` and `apfloat` agree on the interpretation of a floating-point number literal, in case either of them has any bugs left around. r? @nikomatsakis f? @nagisa @est31 <hr/> Huge thanks to @edef1c for first demoing usable `APFloat` bindings and to @chandlerc for fielding my questions on IRC about `APFloat` peculiarities (also upstreaming some bugfixes).

bors · 2017-08-05T15:41:30Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 2b82b7e to master...

kennytm · 2017-08-05T15:48:39Z

cc #43535.

coder543 · 2017-08-09T16:42:18Z

@eddyb This is another vote to have apfloat be a crates.io library. huonw/float is nice, but it depends on ramp, which seems to require nightly, and it looks like float itself might depend on nightly. Would apfloat require nightly?

It's also odd to name the library arbitrary precision float if it isn't truly arbitrary precision.

eddyb · 2017-08-09T18:33:09Z

The IEEE support is configurable (but I chose to use types instead of runtime config), even from outside the crate - you can quite easily create a 24-bit IEEE-like floating-point type with it.
Also, I kept the name from LLVM.

I have nothing against it being on crates.io but that means more work on its API to be more ergonomic, and placing it in the nursery, which @rust-lang/libs gets to decide on.

ramp doesn't have opt-out for the nightly requirement? I thought it did.
As for this library, give me a minute to look at its nightly feature requirements.

eddyb · 2017-08-09T18:36:32Z

src/librustc_apfloat/lib.rs

+#![feature(const_fn)]
+#![feature(i128_type)]
+#![feature(slice_patterns)]
+#![feature(try_from)]


Out of these 4 nightly features, the only hard one to remove is i128_type. The non-nightly version would have to be limited to 64-bit floats instead of supporting 128-bit (IEEE and PPC).

coder543 · 2017-08-09T19:13:23Z

ramp doesn't have opt-out for the nightly requirement? I thought it did.

From the ramp README:

NOTE Due to use of unstable features (notably inline assembly), Ramp can only be compiled with a nightly build of rustc.

So, I don't think it does.

Out of these 4 nightly features, the only hard one to remove is i128_type.

Hopefully that'll be stabilized soonish.

eddyb · 2017-08-09T19:17:33Z

Yeah for some reason when I looked at ramp's code, the assembly seemed opt-out.

alexcrichton · 2017-08-12T20:04:41Z

@eddyb it looks like this caused a 30% regression in the tuple-stress test, do you know if that's a fluke on perf or perhaps related to this?

alexcrichton · 2017-08-12T20:53:57Z

Er I went ahead and opened a dedicated issue for this -- #43828

eddyb added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jul 30, 2017

eddyb requested a review from nikomatsakis July 30, 2017 07:27

rust-highfive assigned nikomatsakis Jul 30, 2017

eddyb removed the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jul 30, 2017

eddyb commented Jul 30, 2017

View reviewed changes

eddyb force-pushed the apfloat branch from 79d1aef to 08d9e09 Compare July 30, 2017 22:13

carols10cents added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jul 31, 2017

nikomatsakis approved these changes Jul 31, 2017

View reviewed changes

eddyb mentioned this pull request Aug 1, 2017

f32/f64 MIN_EXP is either wrong or badly documented. #43583

Closed

eddyb added 4 commits August 2, 2017 03:45

rustc_apfloat: introduce the base Float API.

877ec94

rustc_apfloat: stub IEEE & PPC implementations.

2686a7a

rustc_apfloat: port the suite of unit tests.

7a5fccf

rustc_apfloat: complete the IEEE & PPC implementations.

1409d20

eddyb force-pushed the apfloat branch from 08d9e09 to 18cdb63 Compare August 2, 2017 00:46

eddyb added 2 commits August 2, 2017 17:28

rustc_const_math: use apfloat::ieee::{Single,Double} in ConstFloat.

9861df4

rustc_trans: do not pass floating-point values to LLVM through FFI.

c457b26

eddyb force-pushed the apfloat branch from 18cdb63 to c457b26 Compare August 2, 2017 14:28

eddyb mentioned this pull request Aug 4, 2017

Share the Rust APFloat port with the Rust compiler. bytecodealliance/cranelift#136

Closed

bors merged commit c457b26 into rust-lang:master Aug 5, 2017

eddyb deleted the apfloat branch August 5, 2017 15:49

Frederick888 mentioned this pull request Aug 6, 2017

Failed to build with rustc 1.21.0-nightly (ed16b0a1d 2017-08-05) rust-lang/rust-clippy#1928

Closed

dwrensha added a commit to dwrensha/seer that referenced this pull request Aug 6, 2017

Update for upstream changes: rust-lang/rust#43554

ae20405

eddyb commented Aug 9, 2017

View reviewed changes

alexcrichton mentioned this pull request Aug 12, 2017

30% regression in compile time of tuple-stress benchmark #43828

Closed

APFloat: Rewrite It In Rust and use it for deterministic floating-point CTFE. #43554

APFloat: Rewrite It In Rust and use it for deterministic floating-point CTFE. #43554

Conversation

eddyb commented Jul 30, 2017 • edited Loading

eddyb commented Jul 30, 2017 • edited Loading

eddyb commented Jul 30, 2017

Mark-Simulacrum commented Jul 30, 2017

eddyb commented Jul 30, 2017 • edited Loading

Mark-Simulacrum commented Jul 30, 2017

alexcrichton commented Jul 30, 2017

eddyb commented Jul 30, 2017

eddyb Jul 30, 2017

Choose a reason for hiding this comment

alexcrichton Jul 30, 2017

Choose a reason for hiding this comment

RalfJung commented Jul 30, 2017

eddyb commented Jul 30, 2017

eddyb commented Jul 30, 2017

bors commented Jul 30, 2017

bors commented Jul 30, 2017

Mark-Simulacrum commented Jul 31, 2017

nikomatsakis left a comment

Choose a reason for hiding this comment

eddyb commented Aug 2, 2017

eddyb commented Aug 2, 2017

aturon commented Aug 2, 2017

tomprince commented Aug 5, 2017

eddyb commented Aug 5, 2017 • edited Loading

eddyb commented Aug 5, 2017

bors commented Aug 5, 2017

bors commented Aug 5, 2017

bors commented Aug 5, 2017

bors commented Aug 5, 2017

eddyb commented Aug 5, 2017

bors commented Aug 5, 2017

bors commented Aug 5, 2017

kennytm commented Aug 5, 2017

coder543 commented Aug 9, 2017

eddyb commented Aug 9, 2017

eddyb Aug 9, 2017

Choose a reason for hiding this comment

coder543 commented Aug 9, 2017

eddyb commented Aug 9, 2017

alexcrichton commented Aug 12, 2017

alexcrichton commented Aug 12, 2017

eddyb commented Jul 30, 2017 •

edited

Loading

eddyb commented Jul 30, 2017 •

edited

Loading

eddyb commented Jul 30, 2017 •

edited

Loading

eddyb commented Aug 5, 2017 •

edited

Loading