Ofast deprecation clarifications #101005
Ofast deprecation clarifications #101005
Conversation
|
@llvm/pr-subscribers-clang Author: Sjoerd Meijer (sjoerdmeijer) ChangesFollowing up on the RFC discussion, this is clarifying that the main purpose and effect of the -Ofast deprecation is to discourage its usage and that everything else is more or less open for discussion, e.g. there is no timeline yet for removal. Full diff: https://github.com/llvm/llvm-project/pull/101005.diff 2 Files Affected:
diff --git a/clang/docs/CommandGuide/clang.rst b/clang/docs/CommandGuide/clang.rst
index 663aca1f6ddcb..6ce340b20c252 100644
--- a/clang/docs/CommandGuide/clang.rst
+++ b/clang/docs/CommandGuide/clang.rst
@@ -429,8 +429,11 @@ Code Generation Options
:option:`-Ofast` Enables all the optimizations from :option:`-O3` along
with other aggressive optimizations that may violate strict compliance with
- language standards. This is deprecated in favor of :option:`-O3`
- in combination with :option:`-ffast-math`.
+ language standards. This is deprecated in Clang-19 and a warning is emitted
+ that :option:`-O3` in combination with :option:`-ffast-math` should be used
+ instead if the request for non-standard math behavior is intended. Thus, as
+ there is no timeline yet for removal, the aim is to discourage its usage
+ due to the compliance violating optimizations.
:option:`-Os` Like :option:`-O2` with extra optimizations to reduce code
size.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 71d615553c613..430fa77218954 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -495,10 +495,12 @@ New Compiler Flags
Deprecated Compiler Flags
-------------------------
-- The ``-Ofast`` command-line option has been deprecated. This option both
- enables the ``-O3`` optimization-level, as well as enabling non-standard
- ``-ffast-math`` behaviors. As such, it is somewhat misleading as an
- "optimization level". Users are advised to switch to ``-O3 -ffast-math`` if
+- The ``-Ofast`` command-line option has been deprecated, but there is no
+ timeline for removal yet. Thus, the main effect of emitting a deprecation
+ warning message is to discourage its usage due to the problems of ``-Ofast``:
+ it enables both the ``-O3`` optimization-level as well as non-standard
+ ``-ffast-math`` behaviors and as such it is perceived to be misleading as an
+ optimization level. Users are advised to switch to ``-O3 -ffast-math`` if
the use of non-standard math behavior is intended, and ``-O3`` otherwise.
See `RFC <https://discourse.llvm.org/t/rfc-deprecate-ofast/78687>`_ for details.
|
There was a problem hiding this comment.
Thank you for working on this!
I think we need these changes to be against main and not 19.x, aside from the release notes (otherwise we're losing the documentation in 20.x). For the release notes, I think we can talk to @tstellar and @tru about getting the 19.x notes updated once we've finalized the wording for it.
One other change we should probably make at the same time is to update Options.td to use HelpTextForVariants so the help text for Flang does not suggest the option is deprecated in their driver.
clang/docs/CommandGuide/clang.rst
Outdated
| instead if the request for non-standard math behavior is intended. Thus, as | ||
| there is no timeline yet for removal, the aim is to discourage its usage | ||
| due to the compliance violating optimizations. |
There was a problem hiding this comment.
| instead if the request for non-standard math behavior is intended. Thus, as | |
| there is no timeline yet for removal, the aim is to discourage its usage | |
| due to the compliance violating optimizations. | |
| instead if the request for non-standard math behavior is intended. | |
| There is no timeline yet for removal; the aim is to discourage use of :option:`-Ofast` | |
| due to the surprising behavior of an optimization flag changing the observable | |
| behavior of correct code. | |
clang/docs/ReleaseNotes.rst
Outdated
| - The ``-Ofast`` command-line option has been deprecated, but there is no | ||
| timeline for removal yet. Thus, the main effect of emitting a deprecation | ||
| warning message is to discourage its usage due to the problems of ``-Ofast``: | ||
| it enables both the ``-O3`` optimization-level as well as non-standard | ||
| ``-ffast-math`` behaviors and as such it is perceived to be misleading as an | ||
| optimization level. Users are advised to switch to ``-O3 -ffast-math`` if |
There was a problem hiding this comment.
| - The ``-Ofast`` command-line option has been deprecated, but there is no | |
| timeline for removal yet. Thus, the main effect of emitting a deprecation | |
| warning message is to discourage its usage due to the problems of ``-Ofast``: | |
| it enables both the ``-O3`` optimization-level as well as non-standard | |
| ``-ffast-math`` behaviors and as such it is perceived to be misleading as an | |
| optimization level. Users are advised to switch to ``-O3 -ffast-math`` if | |
| - The ``-Ofast`` command-line option has been deprecated, but there is no | |
| timeline for removal. Deprecation is intended to discourage use of the flag | |
| due to the problems of ``-Ofast``: it enables both the ``-O3`` optimization | |
| level as well as non-standard ``-ffast-math`` behaviors and as such it is | |
| misleading as an optimization level. Users are advised to switch to | |
| ``-O3 -ffast-math`` if |
Oh right, yes, missed that, makes sense. Will split this up, let's do the documentation update here first as suggested. |
Following up on the RFC discussion, this is clarifying that the main purpose and effect of the -Ofast deprecation is to discourage its usage and that everything else is more or less open for discussion, e.g. there is no timeline yet for removal.
sjoerdmeijer
requested review from
tbaederr,
Endilll,
nikic and
a team
as code owners
Co-authored-by: Aaron Ballman <[email protected]>
Co-authored-by: Vlad Serebrennikov <[email protected]>
|
@AaronBallman, shall we merge this? |
Thanks for your help with this. I am mostly unfamiliar with the release process. Do you know how we can achieve this? Do we suggest this commit to the release manager for a backport and cherry pick? Or do we prepare a patch? We could add similar clarifications to the release notes, which were initially included in an earlier version of this patch. So same question how we would need to approach that. |
|
We have instructions at https://www.llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches but the basic gist of it is to add a special comment and add the PR to the correct milestone. As in: /cherry-pick 48d4d4b |
In terms of updating the release notes, once the cherry-pick happens, there's an automated bot message asking if you want to add a release note for the changes. I think the best way forward would be to follow those instructions and ask the release managers to help apply the changes. |
Following up on the RFC discussion, this is clarifying that the main purpose and effect of the -Ofast deprecation is to discourage its usage and that everything else is more or less open for discussion, e.g. there is no timeline yet for removal. --------- Co-authored-by: Aaron Ballman <[email protected]> (cherry picked from commit 48d4d4b)
|
/pull-request #101663 |
Following up on the RFC discussion, this is clarifying that the main purpose and effect of the -Ofast deprecation is to discourage its usage and that everything else is more or less open for discussion, e.g. there is no timeline yet for removal. --------- Co-authored-by: Aaron Ballman <[email protected]>
Following up on the RFC discussion, this is clarifying that the main purpose and effect of the -Ofast deprecation is to discourage its usage and that everything else is more or less open for discussion, e.g. there is no timeline yet for removal.