I don’t really have a stake in the overall discussion here, but I do think it’s it’s very unfortunate that a new major deprecation was landed days before LLVM 19 is branched. This is the kind of change that should go in directly after branching, not directly before branching.
It’s a deprecation, not a behavioral change; putting it in before the branch date was intentional because we want to gather feedback during the initial rc (we get some very targeted feedback on trunk but we get broader feedback on release candidates, such as from distro maintainers).
I am wondering about the " -Ofast
will eventually be removed" part in conjunction with “Coordination with GCC would be appreciated but is not a requirement”: the warning and “soft deprecation” seems valuable, but unless GCC also drops the flag, why would we remove it entirely?
Aren’t we gonna make it just harder for some folks to adopt clang?
What is the pros/cons analysis here? (we can revisit this when we actually try to remove the flag, but it seems worth clarifying this now as well)
Good question!
On the one hand, Clang is not a drop-in replacement for GCC and our command line options are already different (both compilers have options the other compiler does not have and we have options spelled the same way but behave differently), so there’s plenty of precedent for differences with command line flags.
On the other hand, our optimization flags specifically are typically more uniform than say -f
flags, and so it’s a reasonable expectation for both compilers to support the same spellings since they historically have done so.
So I think the outcome of GCC intentionally keeping the flag would either be the status quo (deprecation, eventual removal) or for someone to post an RFC proposing that Clang undeprecates the flag and instead issues a “by the way this probably does things you don’t expect” warning.
It’s also worth remembering that the deprecation period is suggested at two years but it could be much longer if removal is disruptive but still warranted. The deprecation is still useful even if very long-lived; it discourages new uses of the flag in favor of more explicit options.
Correcting myself, actually: they aren’t aliases unfortunately. For example -ffast-math
defines the preprocessor macro __FAST_MATH__
and __FINITE_MATH_ONLY__
(and the finite-math mode) while -funsafe-math-optimizations
does not.
I understand all of this, and I agree with the badly named option.
The main problem I have with taking away an existing option is the compatibility problems and questions we will get about “GCC does this and Clang doesn’t”. This is the “moving the gun from one foot to the other”.
I haven’t mentioned build systems, scripts, documentation, etc. that are real issues but that will disappear over time, but incompatible options don’t.
We should emit the warning, and only deprecate if GCC agrees.
I rather see this as raising awareness of this aspect of -Ofast
in both compilers. Which is a good thing.
I think this will cause breakages if people do warning-free builds using -Werror
, and I believe those people are already aware that changing compilers (or compiler versions) is not effortless.
@AaronBallman : can we revert this just on the basis that doing this just before branching isn’t ideal?
Next steps:
- can we try to reland this early in the llvm20 dev cycle,
- but try to get some alignment with the GCC community in the meantime,
- if this alignment fails, can we consider trying to find consensus for changing the patch to emitting a warning “do you know -Ofast does this and that, and might not be what you want, use xyz instead”?
FWIW my reading of the thread (and why I didn’t feel the need to comment) mostly agrees with this, except I didn’t see consensus for “The -Ofast
will eventually be removed.”. I for one am strongly against removing the flag if gcc does not, as that just makes us incompatible in a way that will inconvenience users for no real benefit.
That said, the congratulatory warning as implemented in [clang] Add deprecation warning for `-Ofast` driver option by Endilll · Pull Request #98736 · llvm/llvm-project · GitHub is fine with me.
This situation already exists today; -Ofast
is one drop in a very large bucket of incompatible command line option differences between Clang and GCC.
I would agree with you if the community’s decision was to make -Ofast
behave differently than it does in GCC because that kind of silent difference is significantly painful for users to discover. But with our current approach, there’s a loud deprecation for a long period of time to allow users to migrate to the improved form before we ever remove the option and create an actual incompatibility, and there’s a straightforward change users can make to get identical behavior as before (so we’re not removing functionality).
There are no painless deprecations, so some amount of friction is expected.
That approach was considered in this RFC and the community’s consensus position was to deprecate regardless of what GCC does…)
Currently, I would be opposed to a revert.
-
The RFC has consensus and there is no new information that wasn’t already discussed regarding whether we should deprecate at all. The consensus position was for deprecation.
-
There is no divergence of behavior with GCC; the behavior of the flag has not changed. Further, the consensus position is that alignment with GCC is not a requirement for the deprecation.
-
Landing this before the branch date was intentional in order to get feedback from a more broad selection of users than we reach on trunk. Delaying that feedback does not seem to get us anything except more time for coordination with GCC, but there’s no need to move in lock-step with GCC regarding the deprecation.
Eventual removal requires its own RFC, so if Clang deprecates and GCC elects not to, we can determine whether we want to continue down the path or change courses. If we get significant feedback during the early RCs that suggests this is too disruptive, we can determine whether we want to continue down the path or change courses.
I wanted to add the following as an argument, but didn’t, so am happy @bogner had the same understanding and added wrote this:
It looks to me that there’s at least one reason for a revert.
I am not sure about this, because I am not sure this was very explicitly discussed. I feel that in this RFC some opinions like this are presented as consensus while that is not the case. I might even agree that in general it is not required, but have we even tried to get this alignment? If not, that is very disappointing, isn’t it?
So based on this, can we revert this?
I did, but I also think “eventually” is open-ended (could be just before the heat death of the universe). Regardless, my expectation is that removal goes through its own RFC process and is debated on its own merits. Actually, let me be more clear and just say outright: removal requires its own RFC; the consensus was about the intent to try to remove it someday, but it wasn’t consensus to remove after two years without discussion. For example, removal could be “we no longer document the option but still support it for backwards compatibility and keep the deprecation diagnostic forever.”
I suspect you won’t be alone in that position.
and others touched on it at least tangentially. It is especially worth noting the last link where @jcranmer pointed out that -ffast-math
is different between Clang and GCC, so the options are not actually compatible to begin with.
Calling consensus is not a science and reasonable folks can disagree with how it gets called. However, it doesn’t change the fact that I called consensus over a month ago and people spent their time working on the patch on that go-ahead.
If you think consensus was called improperly, then I’m sorry for that outcome, but I also don’t think that conclusion is up for debate because RFC authors need some signal to move forward with changes and that isn’t going to be reversed later without significant technical justification and they got that signal. (In other news, this is a great chance for me to remind folks about the RFC for a governance proposal which helps to resolve this kind of situation. So hopefully we’ll have improved processes in the relatively near future!)
The only mention I could find on the GCC mailing list was:
[Bug middle-end/97263] For -ffinite-math-only -OFast is not mentioned., so they’re aware but aren’t yet discussing whether to follow suit or not. It would be reasonable for someone to raise the question on their mailing list more explicitly, but in the interim, I filed: 115990 – Consider deprecating -Ofast
I still do not agree with a revert at this time; my position is that we should leave this in Clang 19 and reassess if we get feedback on this change during the RCs.
[Bug middle-end/97263] For -ffinite-math-only -OFast is not mentioned.
https://gcc.gnu.org/pipermail/gcc-bugs/2024-May/863833.html, so
they’re aware but aren’t yet discussing whether to follow suit or not.
It would be reasonable for someone to raise the question on their
mailing list more explicitly, but in the interim, I filed: 115990 –
Consider deprecating -Ofast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115990
It’s worth pointing out that in the discussion that’s already happened
on the gcc bug, it’s pointed out that -Ofast
in gcc does more than
-O3 -ffast-math
, yet another difference between gcc and clang.
In looking up the clang source code in response to that comment, I see
that -Ofast
isn’t -O3 -ffast-math
in clang either, it’s actually
-O3 -ffast-math -fstrict-aliasing
. If even the compiler authors aren’t
aware of all the ramifications of the -Ofast
aren’t known by the
compiler authors, let alone user documentation, then perhaps it’s really
not a good option.
The GCC issue I filed already got comments on it from @pinskia (thank you!) and there’s two things worth noting:
- They’re unlikely to deprecate and even more unlikely to remove, at least based on the feedback thus far
-Ofast
is already more incompatible between Clang and GCC than seems to be understood in this thread, beyond-ffast-math
differences, such as enabling the unrolling of loops, and more.
Oh boy, this gets worse the more you look at it. I wonder how many people know that -fno-strict-aliasing -Ofast
enables strict aliasing while -Ofast -fno-strict-aliasing
leaves it disabled; that was certainly a surprise to me.
Compiler Explorer (search the output for alias
and note the missing -relaxed-aliasing
in the bottom compiler invocation that exists in the top compiler invocation.)
Ofast enabling strict aliasing is a surprise to me too. It is consistent with Ofast meaning “make it faster and wronger please” though so that seems fine.
The effect where order of commandline flags override previous ones and some flags act as groups containing other flags which can be individually overridden is a nightmare in general. I remember someone saying that every compiler flag is a bug.
That one might be worth addressing in the general case - traverse the commandline, making note of when things are turned on and off again, and print some diagnostics on request along the lines of “this is what you asked for, it might not be what you wanted”.
Thanks @AaronBallman for checking with the GCC community and raising that bugzilla ticket. The outcome is as expected, but now we know for sure, so that is a good outcome.
The fact that this was checked post-mortem (i.e. after the commit) contributes to my overall impression that all of this was pushed and rushed through. I missed the call for last opinions on this RFC which is my own fault, but I think I was not the only given the number of replies after my message. I am hoping for a second chance to discuss this because removing a user-facing option is a big thing:
Okay, can we then discuss an alternative, or a compromise? I.e., why do we need to deprecate this? In other words, can the compromise by emitting a warning, but not deprecating it since GCC has no intentions to do the same?
Can the compromise be emitting a warning, and improve the documentation? Why are we not achieving our goals if we do this instead of deprecating it?
Let me elaborate on the documentation part. In earlier messages I expressed my disappointment that the documentation improvement was not included in this patch. The whole problem description was -Ofast isn’t understood. Yes, the patch to deprecate Ofast came with a one or two line documentation change, but that is not addressing the issue that Ofast is a footgun. If we deprecate Ofast, I then expect clear guidance what exactly the footguns are and what the recommendations are, also in terms of compiler options. Yes, we document the fast math and floating point options, but it doesn’t provide any of this guidance. Floating point without some (sub)set of fast math isn’t great, so people are going to look for it.
Summarising, I understand the problem with the name Ofast, I agree it’s unclear to users, and the other problems too, but none of this means we need to deprecate it. Taking away an option that has existed for 15 years is going to be a compatibility issue that we will have to deal with for many years to come as well as questions like why Clang doesn’t support that option, or why Clang is slower. It is just not an improvement in my opinion.
The fact that it wasn’t done before the consensus was called, despite 27 people participating in the discussion, to me contributes to the impression that GCC compatibility argument is not that important to this community.
I also don’t think there was anything rushed about calling consensus on this RFC. Discussion died down almost a month prior to calling consensus (4 posts between May 9 and June 5), and this thread was dormant for another month after that, until I prepared a PR that implements it.
27 people participated in the discussion before consensus was called, including yourself, which I believe is not an insignificant number by measures of our community, but I see only 3 new participants, and no new arguments strong enough to overthrow the consensus. We should be respectful to people who participated in this discussion in a timely manner, decided not to object when the consensus was called, and who don’t want to rehash the discussion. I believe they are the invisible majority here.
As Aaron explicitly said earlier in this thread, this RFC does not remove the option. We declare it deprecated, and issue a warning. Users can still use -Ofast
, and they can silence the warning like any other warning.
We issue a warning, like you suggest. Declaring it deprecated gives us ability to close bugs about quirks in -Ofast
behavior as something we’re not going to fix.
We (Clang) would welcome the patch that improves our documentation. But this wasn’t considered a blocker in the discussion before the consensus was called.
To be honest, this is a very long RFC and the consensus on this isn’t as clear to me as it seems to be for you. And the comments in this thread since the PR landed seem also an indication of this. Maybe you can expand on what makes you believe that the consensus was there on this particular aspect?
Here are all the quotes I could find in this thread about the specific question of gcc compatibility: