Add target_has_floating_point property and make floats optional in libcore by phil-opp · Pull Request #32651 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation10 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
The first commit adds an optional has_floating_point property (defaulting to true), which can be used for conditional compilation. It's feature gated as cfg_target_has_floating_point.
The second commit uses the new has_floating_point flag to make all floating point uses in libcore optional. This makes it possible to compile a float-free libcore by adding the following entries to the custom-target.json:
"features": "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-3dnow,-3dnowa,-avx,-avx2", "has-floating-point": false,
TODO:
- Add a test that compiles
libcorefor a float-free target and ensures that the“LLVM ERROR: SSE register return with SSE disabled”error does not occur. What's the best way to add this test? - Fix stage0
This adds a new target property, target_has_floating_point which can be used as a matcher for conditional compilation. The default value is true.
Matching against the target_has_floating_point attribute with #[cfg] is currently feature gated as cfg_target_has_floating_point.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.
If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.
Please see the contribution instructions for more information.
I think the f32 and f64 types should be disabled by the compiler when the target doesn't support floating-point, so that any use of these types would result in a compile-time error rather than an LLVM assert.
The documentation should also be updated to say that only libcore is supported in a float-free configuration, and not any of the other standard Rust libraries.
@Amanieu That's going a step too far given the use case (OS development) this PR is intended to solve. There's nothing intrinsically wrong with values of type f32 or f64, nor is there anything stopping the user providing software implementations of operations on floating-point values.
Thanks for the PR @phil-opp! Working with and/or changing the behavior of the primitive types in Rust is a pretty serious PR, though, so this may be breaching requires-an-RFC territory in terms of proposing such a change. (in terms of a formal document rather than an issue itself).
cc @rust-lang/lang
cc @rust-lang/libs
I would personally also expect that this sort of option would disable the floating point types entirely. The problem is that the ABI of floats requires the use of SSE registers, right? (and the purpose of this is to disable use of SSE register?)
Relevant to the language team
Relevant to the library API team, which will review and decide on the PR/issue.
labels
Working with and/or changing the behavior of the primitive types in Rust is a pretty serious PR
Agreed. The design was proposed by @emk in rust-lang/rfcs#1364 (comment). It should only change the behavior for targets that set the new has_floating_point field to false.
I could try to create a RFC if it's needed. However, it seems like there are some issues with the current design:
I would personally also expect that this sort of option would disable the floating point types entirely. The problem is that the ABI of floats requires the use of SSE registers, right?
Yes, I just tried it. A simple function returning a f32 triggers the LLVM SSE error again:
So I agree that it would be better to disable f32 and f64 completely. But is it even possible to disable primitive types?
There's nothing intrinsically wrong with values of type f32 or f64, nor is there anything stopping the user providing software implementations of operations on floating-point values.
I though so, too. But it seems like a simple float return already uses SSE registers…
@phil-opp I stand corrected—I thought it'd happily store an f64 in (e.g.) rax if told not to use SSE registers.
But is it even possible to disable primitive types?
In principle, yeah, although it may be kinda hard in practice. If the compiler just removed the names from the default namespace, there's no way to import them, so you can be pretty sure that nothing uses them. That being said you'd also probably want to verify that dependencies don't use floats as well, but that's just a minor complication.
I just wanted to make a general point that we have a strong need for vision in the area of arch/OS/config/...-dependent APIs.
Examples include:
- Floating point types, as per this PR
- SIMD capabilities
- Including dynamic detection
- Atomics
- Android compatiblity issues (with breaking ABI at various releases)
The common thread among all these problems is that we want to expose, in libraries like std, sets of APIs whose availability may depend on various aspects of a platform (OS, architecture, configuration) statically or dynamically. The only place we currently do this is std::os::your_os_here, where each OS's submodule is linked to a cfg. The rationale was that it was easy to audit for non-cross-platform code -- just look for std::os::*.
But this clearly doesn't scale to other kinds of distinctions.
Now, it's not obvious that all of the above examples can be solved by a single mechanism. But I am really eager to see some basic vision for how to approach these questions in Rust, before we go too far down the road of adding specific ad hoc APIs.
If @phil-opp or anyone else is interested in this topic, I'd love to work together toward an RFC!
The libs team discussed this during triage yesterday and we agreed that this likely needs an RFC before moving forward, so I'm going to close this for now. Feel free to reach out to me or @aturon though @phil-opp as we'd both be quite interested in helping out with an RFC!
Labels
Relevant to the language team
Relevant to the library API team, which will review and decide on the PR/issue.