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

Add target_has_floating_point property and make floats optional in libcore #32651

Closed
wants to merge 2 commits into from

Conversation

phil-opp
Copy link
Contributor

See rust-lang/rfcs#1364

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 libcore for 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`.
@rust-highfive
Copy link
Collaborator

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.

@Amanieu
Copy link
Member

Amanieu commented Mar 31, 2016

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.

@ketsuban
Copy link
Contributor

@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.

@alexcrichton
Copy link
Member

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?)

@alexcrichton alexcrichton added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 1, 2016
@phil-opp
Copy link
Contributor Author

phil-opp commented Apr 1, 2016

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:

fn foo() -> f32 {
    32.0
}

So I agree that it would be better to disable f32 and f64 completely. But is it even possible to disable primitive types?

@phil-opp
Copy link
Contributor Author

phil-opp commented Apr 1, 2016

@ketsuban

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…

@ketsuban
Copy link
Contributor

ketsuban commented Apr 1, 2016

@phil-opp I stand corrected—I thought it'd happily store an f64 in (e.g.) rax if told not to use SSE registers.

@alexcrichton
Copy link
Member

@phil-opp

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.

@aturon
Copy link
Member

aturon commented Apr 4, 2016

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!

@alexcrichton
Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants