Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upRemove the quad_precision_float feature gate #15160
Conversation
This comment has been minimized.
This comment has been minimized.
|
This feature is fully usable today, I don't think there's any additional compiler support required. It's feature gated so there's no harm in having it available for use with the libgcc_s (libquadmath) f128 support. It's a feature offered by Clang, GCC, ICC and MSVC and works fine with all of them out-of-the-box. The support is compiler-rt is maturing, and it now supports addition, subtraction, multiplication and has an initial (but buggy) comparison implementation. It will soon have full support for the required operations on 64-bit without needing libgcc_s. http://reviews.llvm.org/rL211312 AFAIK, this is exactly what feature gates are for and there's hardly any cost to supporting an extra floating point case in the compiler. I don't buy the argument that it's somehow detrimental to the long-term development, it's just a few lines where there's code handling f32 and f64. |
This comment has been minimized.
This comment has been minimized.
|
I agree with @thestinger. While I have very little knowledge about f128, it does seem like this is precisely what feature gates are really meant for. Unless the feature has been explicitly rejected, removing it with the rationale that it's not fully-implemented doesn't make sense. |
This comment has been minimized.
This comment has been minimized.
brson
commented on 3d308fe
Jun 25, 2014
|
r+ This feature has been explicitly rejected: https://github.com/rust-lang/meeting-minutes/blob/master/weekly-meetings/2014-06-24.md#removing-f128 |
This comment has been minimized.
This comment has been minimized.
|
I recommend reading today's minutes about this topic: https://github.com/rust-lang/meeting-minutes/blob/master/weekly-meetings/2014-06-24.md#removing-f128, there are some compelling arguments for why today's state isn't quite desired. |
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton Ok, that makes sense. It wasn't clear from the PR description that it was decided that this was both unnecessary and unlikely to be fixed. |
This comment has been minimized.
This comment has been minimized.
|
It's a well-defined part of the ABI on x86_64, PowerPC, ARMv8 and some other architectures. It's fully supported by LLVM too. It already completely works today with libgcc_s, and will soon work with only compiler-rt. The fact that it was rejected in a meeting doesn't make it any less of a stupid decision. |
This comment has been minimized.
This comment has been minimized.
|
It's just yet another reason why C / C++ are better languages than Rust. Rust's feature set is determined by arbitrary political decisions not based on facts. |
This comment has been minimized.
This comment has been minimized.
|
@thestinger I would appreciate it if you kept comments like that to yourself. That's not productive, and it serves no purpose other than to be inflammatory. In addition, if you care about this feature, perhaps you should implement full support for it? If Rust had enough support for f128 to actually do numerics code (as referenced in the meeting notes), then it would make more sense to keep around. |
This comment has been minimized.
This comment has been minimized.
|
@thestinger I realize you are unhappy, but please refrain from such accusations. The reasoning has been spelled out. You are welcome to disagree. |
This comment has been minimized.
This comment has been minimized.
|
@kballard: The compiler already has full support for this. I didn't add support for passing it to foreign functions, but that's really easy to do. Can anyone point out what support is actually missing for this, and exactly what doesn't already work? @brson: The reasoning has not been spelled out. The meeting notes only contain inaccuracies, and the real reason is an arbitrary political choice. It's not different than deciding to reject a pull request implementing tail call elimination support behind a feature gate. The claim that a few dozen lines of code following the same pattern as A highly controversial, immature feature without a clearly laid out design like |
This comment has been minimized.
This comment has been minimized.
|
can we have sources for all of these comments? All i read is a lot of anger and no fact |
This comment has been minimized.
This comment has been minimized.
zwarich
commented
Jun 25, 2014
|
@thestinger, isn't the whole point of the RFC process to discuss these things through the RFC process rather than the issue tracker? Anyways, it seems that the non-Apple AArch64 LLVM backend does now include |
This comment has been minimized.
This comment has been minimized.
This isn't an RFC. |
This comment has been minimized.
This comment has been minimized.
|
@sinistersnare: A program using #![feature(quad_precision_float)]
fn main() {
let x: f128 = 5.5;
let y = x * 2.0 - 10.0;
println!("{}", y as f64)
}The library support wasn't written yet, so you can't pass it to |
This comment has been minimized.
This comment has been minimized.
zwarich
commented
Jun 25, 2014
|
@huonw, I corrected that typo right after I posted it. Sorry about that! |
This comment has been minimized.
This comment has been minimized.
|
@thestinger I understand that f128 works on x86_64, but where is the source where it works on all of the compilers you said it works on, on all of the architectures that at least LLVM works on? I dont doubt you, but you just saying so will not prove your point. |
This comment has been minimized.
This comment has been minimized.
|
The lack of a complete implementation for all architectures in LLVM is why this was landed behind a feature gate. I don't know why Rust has feature gates if not for a use case like this where a feature is useful but not yet something where availability can be guaranteed everywhere. The support in both compiler-rt and LLVM is being worked on. @sinistersnare: You can verify that it's implemented in those compilers with a quick search. There's also a |
This comment has been minimized.
This comment has been minimized.
|
For what its worth I agree with you @thestinger and I think this should be kept behind a feature gate, and stay in the tree. I am not really a fan of your attitude, but I can look past that and see what you mean. I have verified that most implementations support long double, but basically as a different way to just say double, whereas llvm has partial support; on x86 it is 80 bits, not 128. |
This comment has been minimized.
This comment has been minimized.
|
I'm not going to have a positive attitude when falsehoods are used to justify removing a feature I care about. There are two right there in the commit message claiming it is "half baked" and that it will somehow be a burden to support. If the justification for removing it was "we don't like it or see the need for it" then at least it would be honest and I could agree to disagree. |
This comment has been minimized.
This comment has been minimized.
|
@sinistersnare: GCC has a __float128 type. The |
This comment has been minimized.
This comment has been minimized.
|
saw approval from brson |
This comment has been minimized.
This comment has been minimized.
|
merging alexcrichton/rust/remove-f128 = 3d308fe into auto |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
fast-forwarding master to auto = 7a93bee |
bors
added a commit
that referenced
this pull request
Jun 25, 2014
This comment has been minimized.
This comment has been minimized.
|
libquadmath (LGPL) supports all libm functions for f128, so having full f128 supports seems to only be a matter of providing an implementation of Float that calls them. Just calling strtoflt128 should allow to easily parse f128 literals, and you can print it with quadmath_snprintf (I suppose MPFR would work as well). I guess the idea of this change is to remove the thing until someone codes the above, though. |
This comment has been minimized.
This comment has been minimized.
|
f128 should have went through the RFC process in the first place. A feature, and feature gate in general, shouldn't be added without RFC. |
This comment has been minimized.
This comment has been minimized.
|
@bill-myers: I don't get the impression that it's being removed until library support is finished. I doubt that the fact that I have f128 work including a fair bit of library support in a local branch matters at all. |
This comment has been minimized.
This comment has been minimized.
|
@cmr: So is there a list of people excluded from that rule anywhere? I guess anything goes when it wasn't the unpaid help working on it. |
This comment has been minimized.
This comment has been minimized.
|
fwiw I also disagree with that one. |
bors
closed this
Jun 25, 2014
bors
merged commit 3d308fe
into
rust-lang:master
Jun 25, 2014
2 checks passed
This comment has been minimized.
This comment has been minimized.
asb
commented
Jun 25, 2014
|
Really this comes down to a policy question - what is the feature gate for? I would have thought the feature gate was to allow features like quad precision float to mature. Perhaps there's now a policy that gated features should be "complete", but that's not how it's been used historically. The removal seems odd to me as well given 1) it's a tiny amount of code 2) it's hard to imagine a future where Rust would not at some point gain f128 support. |
This comment has been minimized.
This comment has been minimized.
ghost
commented
Jun 25, 2014
|
I am biased because i work in computational science. f128 is needed for the science community and many other communities i imagine. |
This comment has been minimized.
This comment has been minimized.
|
I must loudly disagree with the notion that feature-gated things are exempt from the RFC process. Would you justify the removal of |
This comment has been minimized.
This comment has been minimized.
|
To remind everyone, the furor over unilateral and insufficiently-discussed features such as this one are what prompted the RFC process in the first place. Not enough people were given the opportunity to voice objections to the removal of this feature, and now the developers are going to pay the price via loss of goodwill from the community. |
alexcrichton
deleted the
alexcrichton:remove-f128
branch
Jun 25, 2014
This comment has been minimized.
This comment has been minimized.
alexchandel
commented
Jun 25, 2014
|
This is a terrible shame. Native support for quad precision floats made Rust special to me, like Fortran's
|
This comment has been minimized.
This comment has been minimized.
|
On Wed, Jun 25, 2014 at 4:43 PM, Alex notifications@github.com wrote:
|
This comment has been minimized.
This comment has been minimized.
alexchandel
commented
Jun 26, 2014
|
From @cmr 's brief RFC, adding a built-in |
This comment has been minimized.
This comment has been minimized.
|
There's no more compiler support for |
This comment has been minimized.
This comment has been minimized.
alexchandel
commented
Jun 26, 2014
|
I meant LLVM supports |
alexcrichton commentedJun 24, 2014
The f128 type has very little support in the compiler and the feature is
basically unusable today. Supporting half-baked features in the compiler can be
detrimental to the long-term development of the compiler, and hence this feature
is being removed.