[clang] Add deprecation warning for `-Ofast` driver option #98736

Endilll · 2024-07-13T10:13:55Z

This patch implements consensus on the corresponding RFC documented here: https://discourse.llvm.org/t/rfc-deprecate-ofast/78687/72
Specifically, I added a deprecation warning for -Ofast, that suggests to use -O3 or -O3 with -ffast-math, and a new diagnostic group for aforementioned warning.
Deprecation period is going to be lengthy, so I hope this PR can be merged in time for Clang 19.

llvmbot · 2024-07-13T10:14:12Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Vlad Serebrennikov (Endilll)

Changes

This patch implements consensus on the corresponding RFC documented here: https://discourse.llvm.org/t/rfc-deprecate-ofast/78687/72
Specifically, I added a deprecation warning for -Ofast, that suggests to use -O3 or -O3 with -ffast-math, and a new diagnostic group for aforementioned warning.

Full diff: https://github.com/llvm/llvm-project/pull/98736.diff

6 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+3)
(modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+3)
(modified) clang/include/clang/Basic/DiagnosticGroups.td (+2)
(modified) clang/include/clang/Driver/Options.td (+2-1)
(modified) clang/lib/Driver/ToolChains/Clang.cpp (+2)
(modified) clang/test/Driver/Ofast.c (+4)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ceead8c362e9c..2f5f40f355901 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -470,6 +470,9 @@ New Compiler Flags
 Deprecated Compiler Flags
 -------------------------
 
+- ``-Ofast`` is deprecated in favor of ``-O3``, possibly combined with ``-ffast-math``.
+  See `RFC <https://discourse.llvm.org/t/rfc-deprecate-ofast/78687>`_ for details.
+
 Modified Compiler Flags
 -----------------------
 - Added a new diagnostic flag ``-Wreturn-mismatch`` which is grouped under
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 359c0de7f811c..3857f2f3a33ce 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -442,6 +442,9 @@ def warn_drv_deprecated_arg : Warning<
 def warn_drv_deprecated_arg_no_relaxed_template_template_args : Warning<
   "argument '-fno-relaxed-template-template-args' is deprecated">,
   InGroup<DeprecatedNoRelaxedTemplateTemplateArgs>;
+def warn_drv_deprecated_arg_ofast : Warning<
+  "argument '-Ofast' is deprecated; use '-O3', possibly with '-ffast-math'">,
+  InGroup<DeprecatedOFast>;
 def warn_drv_deprecated_custom : Warning<
   "argument '%0' is deprecated, %1">, InGroup<Deprecated>;
 def warn_drv_assuming_mfloat_abi_is : Warning<
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 2241f8481484e..d7dba76a0fcf8 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -103,6 +103,7 @@ def EnumConversion : DiagGroup<"enum-conversion",
                                 EnumFloatConversion,
                                 EnumCompareConditional]>;
 def DeprecatedNoRelaxedTemplateTemplateArgs : DiagGroup<"deprecated-no-relaxed-template-template-args">;
+def DeprecatedOFast : DiagGroup<"deprecated-ofast">;
 def ObjCSignedCharBoolImplicitIntConversion :
   DiagGroup<"objc-signed-char-bool-implicit-int-conversion">;
 def Shorten64To32 : DiagGroup<"shorten-64-to-32">;
@@ -228,6 +229,7 @@ def Deprecated : DiagGroup<"deprecated", [DeprecatedAnonEnumEnumConversion,
                                           DeprecatedPragma,
                                           DeprecatedRegister,
                                           DeprecatedNoRelaxedTemplateTemplateArgs,
+                                          DeprecatedOFast,
                                           DeprecatedThisCapture,
                                           DeprecatedType,
                                           DeprecatedVolatile,
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index f1e8cb87e5321..269790476c1c3 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -931,7 +931,8 @@ def O : Joined<["-"], "O">, Group<O_Group>,
 def O_flag : Flag<["-"], "O">, Visibility<[ClangOption, CC1Option, FC1Option]>,
   Alias<O>, AliasArgs<["1"]>;
 def Ofast : Joined<["-"], "Ofast">, Group<O_Group>,
-  Visibility<[ClangOption, CC1Option, FlangOption]>;
+  Visibility<[ClangOption, CC1Option, FlangOption]>,
+  HelpText<"Deprecated; use -O3, possibly with -ffast-math">;
 def P : Flag<["-"], "P">,
   Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>,
   Group<Preprocessor_Group>,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index bc21d03a627b9..33e7152343171 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5712,6 +5712,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
                      options::OPT_fno_zero_initialized_in_bss);
 
   bool OFastEnabled = isOptimizationLevelFast(Args);
+  if (OFastEnabled)
+    D.Diag(diag::warn_drv_deprecated_arg_ofast);
   // If -Ofast is the optimization level, then -fstrict-aliasing should be
   // enabled.  This alias option is being used to simplify the hasFlag logic.
   OptSpecifier StrictAliasingAliasOption =
diff --git a/clang/test/Driver/Ofast.c b/clang/test/Driver/Ofast.c
index 8b7f2217eca2f..f5a2aabe257f1 100644
--- a/clang/test/Driver/Ofast.c
+++ b/clang/test/Driver/Ofast.c
@@ -10,6 +10,7 @@
 // RUN: %clang -Ofast -fno-strict-aliasing -### %s 2>&1 | FileCheck -check-prefix=CHECK-OFAST-NO-STRICT-ALIASING %s
 // RUN: %clang -Ofast -fno-vectorize -### %s 2>&1 | FileCheck -check-prefix=CHECK-OFAST-NO-VECTORIZE %s
 
+// CHECK-OFAST: warning: argument '-Ofast' is deprecated; use '-O3', possibly with '-ffast-math'
 // CHECK-OFAST: -cc1
 // CHECK-OFAST-NOT: -relaxed-aliasing
 // CHECK-OFAST: -ffast-math
@@ -23,18 +24,21 @@
 // CHECK-OFAST-O2-NOT: -Ofast
 // CHECK-OFAST-O2: -vectorize-loops
 
+// CHECK-OFAST-NO-FAST-MATH: warning: argument '-Ofast' is deprecated; use '-O3', possibly with '-ffast-math'
 // CHECK-OFAST-NO-FAST-MATH: -cc1
 // CHECK-OFAST-NO-FAST-MATH-NOT: -relaxed-aliasing
 // CHECK-OFAST-NO-FAST-MATH-NOT: -ffast-math
 // CHECK-OFAST-NO-FAST-MATH: -Ofast
 // CHECK-OFAST-NO-FAST-MATH: -vectorize-loops
 
+// CHECK-OFAST-NO-STRICT-ALIASING: warning: argument '-Ofast' is deprecated; use '-O3', possibly with '-ffast-math'
 // CHECK-OFAST-NO-STRICT-ALIASING: -cc1
 // CHECK-OFAST-NO-STRICT-ALIASING: -relaxed-aliasing
 // CHECK-OFAST-NO-STRICT-ALIASING: -ffast-math
 // CHECK-OFAST-NO-STRICT-ALIASING: -Ofast
 // CHECK-OFAST-NO-STRICT-ALIASING: -vectorize-loops
 
+// CHECK-OFAST-NO-VECTORIZE: warning: argument '-Ofast' is deprecated; use '-O3', possibly with '-ffast-math'
 // CHECK-OFAST-NO-VECTORIZE: -cc1
 // CHECK-OFAST-NO-VECTORIZE-NOT: -relaxed-aliasing
 // CHECK-OFAST-NO-VECTORIZE: -ffast-math

clang/test/Driver/Ofast.c

jyknight

Thanks for sending this. I had planned to, but hadn't gotten around to it yet.

clang/docs/ReleaseNotes.rst

jyknight · 2024-07-14T15:35:47Z

clang/include/clang/Basic/DiagnosticDriverKinds.td

@@ -442,6 +442,9 @@ def warn_drv_deprecated_arg : Warning<
 def warn_drv_deprecated_arg_no_relaxed_template_template_args : Warning<
  "argument '-fno-relaxed-template-template-args' is deprecated">,
  InGroup<DeprecatedNoRelaxedTemplateTemplateArgs>;
+def warn_drv_deprecated_arg_ofast : Warning<
+  "argument '-Ofast' is deprecated; use '-O3', possibly with '-ffast-math'">,


Maybe worth using a few more words? E.g.
argument '-Ofast' is deprecated; use '-O3' for standard optimization, or '-O3 -ffast-math' if non-standard "fast-math" was desired

I'm not sure if that's too long.

We could shorten slightly it to:
'-Ofast' is deprecated; use '-O3' for conforming optimizations or '-O3 -ffast-math' for nonconforming optimizations"

I think in this diagnostic it's more important to inform the user that he should replace -Ofast with -O3 -ffast-math in order to just circumvent the deprecation and keep the current behavior, and this should be clear. Throwing the option of using just O3 as equally valid is a bit misleading, but we can, as a note, inform the user that it's possible he made the wrong choice in the first place, and advise a switch to O3.

I think @mizvekov raised a good point, so I tried to take it into account.
@jyknight @AaronBallman What do you think of the new wording (use '-O3 -ffast math' instead, or just '-O3' if you don't intend to enable non-conforming optimizations)?

use '-O3 -ffast math' instead, or just '-O3' if you don't intend to enable non-conforming optimizations

This message LGTM.

How about a slight rewording?

use '-O3 -ffast math' for the same behavior, or '-O3' to enable conforming optimizations only

(I mostly want to drop "just" and to make it more clear that -Ofast was non-conforming because of -ffast-math.)

Switched to Aaron's wording

AaronBallman

Thank you for working on this!

AaronBallman · 2024-07-15T14:39:28Z

clang/include/clang/Basic/DiagnosticDriverKinds.td

@@ -442,6 +442,9 @@ def warn_drv_deprecated_arg : Warning<
 def warn_drv_deprecated_arg_no_relaxed_template_template_args : Warning<
  "argument '-fno-relaxed-template-template-args' is deprecated">,
  InGroup<DeprecatedNoRelaxedTemplateTemplateArgs>;
+def warn_drv_deprecated_arg_ofast : Warning<
+  "argument '-Ofast' is deprecated; use '-O3', possibly with '-ffast-math'">,


We could shorten slightly it to:
'-Ofast' is deprecated; use '-O3' for conforming optimizations or '-O3 -ffast-math' for nonconforming optimizations"

clang/include/clang/Driver/Options.td

clang/test/Driver/Ofast.c

cor3ntin

LGTM!

tschuett · 2024-07-16T11:07:42Z

We are deprecating -Ofast and I would prefer to hint users first at conforming behaviour -O3 and then as last resort at -O3 with -ffast-math.

Endilll · 2024-07-16T11:21:31Z

We are deprecating -Ofast and I would prefer to hint users first at conforming behaviour -O3 and then as last resort at -O3 with -ffast-math.

@tschuett I'm sympathetic to this argument, but I think it's no less important to tell users in no uncertain terms how to preserve their current behavior. If there's a wording that achieves both goals, I'd be happy to incorporate it into this PR.

tschuett · 2024-07-16T11:24:06Z

We are deprecating -Ofast and I would prefer to hint users first at conforming behaviour -O3 and then as last resort at -O3 with -ffast-math.

@tschuett I'm sympathetic to this arguments, but I think it's no less important to tell users in no uncertain terms how to preserve their current behavior. If there's a wording that achieves both goals, I'd be happy to incorporate it into this PR.

No worries. I have zero voting rights in Clang.

cor3ntin · 2024-07-16T11:50:51Z

@Endilll I think Aaron's suggestion gets you closer to what @tschuett wants

AaronBallman

LGTM!

Endilll · 2024-07-16T14:34:32Z

@jyknight As the author of the RFC, are you happy with the current state of this PR?

jyknight

Sounds good enough to me, thank you!

kiranchandramohan · 2024-07-16T15:48:53Z

Could you add a flang driver test as well?

Endilll · 2024-07-16T16:50:19Z

Could you add a flang driver test as well?

I have troubles building Flang on my machine to test the change, so I'd prefer this be done in a separate PR by someone who knows how to do this. Is this fine by you?

kiranchandramohan · 2024-07-16T17:38:05Z

Fine with me.

…

On Tue, 16 Jul 2024, 17:50 Vlad Serebrennikov, ***@***.***> wrote: Could you add a flang driver test as well? I have troubles building Flang on my machine to test the change, so I'd prefer this be done in a separate PR by someone who knows how to do this. Is this fine by you? — Reply to this email directly, view it on GitHub <#98736 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4YEDKN4VQV5UOT7QBQOWTZMVFOLAVCNFSM6AAAAABK2EZQH2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZRGM4DSMBVHE> . You are receiving this because you commented.Message ID: ***@***.***>

AaronBallman · 2024-07-16T18:39:51Z

Could you add a flang driver test as well?

The RFC was specifically about -Ofast in Clang and not behavior in Flang, so what test behavior would you like to see? This patch currently deprecates for both Clang and Flang, is that acceptable?

I was asking for a test that checks for the warning in Flang. But if @Endilll cannot build flang, you can proceed with the patch. I will create a patch separately for Flang.

Excellent, thank you for confirming!

clang/include/clang/Driver/Options.td

Endilll · 2024-07-18T07:49:45Z

This PR has passed CI before latest wording suggestion by Shafik, so I'll go ahead and merge this.

sjoerdmeijer · 2024-07-18T12:37:47Z

I will copy here what I wrote on the RFC:

I just noticed that this patch went in. I find it very disappointing to see that the required documentation changes and improvements didn’t start with this patch. In the clang documentation, we now only say that -Ofast is deprecated and that -O3 -ffast-math should be used, so what has changed? This doesn’t explain what the “footgun” is, what the safe subset of fast math flags are, what the unsafe ones are, and their benefits, so I am very unimpressed with all of this.

clang/include/clang/Basic/DiagnosticDriverKinds.td

A follow-up for #98736

nico · 2024-07-22T22:31:09Z

This seems to break tests on my Windows box: http://45.33.8.238/win/91548/step_6.txt

Endilll · 2024-07-23T10:22:14Z

This seems to break tests on my Windows box: http://45.33.8.238/win/91548/step_6.txt

This is the failed test:

c:\src\llvm-project\out\gn\bin\clang.exe -Ofast -O2 -### -Werror C:\src\llvm-project\clang\test\Driver\Ofast.c 2>&1 | c:\src\llvm-project\out\gn\bin\filecheck.exe -check-prefix=CHECK-OFAST-O2    --check-prefix=CHECK-OFAST-O2-ALIASING-MSVC  C:\src\llvm-project\clang\test\Driver\Ofast.c
# executed command: 'c:\src\llvm-project\out\gn\bin\clang.exe' -Ofast -O2 '-###' -Werror 'C:\src\llvm-project\clang\test\Driver\Ofast.c'
# note: command had no output on stdout or stderr
# error: command failed with exit status: 1
# executed command: 'c:\src\llvm-project\out\gn\bin\filecheck.exe' -check-prefix=CHECK-OFAST-O2 --check-prefix=CHECK-OFAST-O2-ALIASING-MSVC 'C:\src\llvm-project\clang\test\Driver\Ofast.c'

Somehow clang.exe -Ofast -O2 -### -Werror C:\src\llvm-project\clang\test\Driver\Ofast.c 2>&1 is not producing any output for you, despite -### being present. I'm not sure why, but all windows buildbots seem to be happy with this patch. Can you investigate further?

CC @AaronBallman @MaskRay

AaronBallman · 2024-07-23T12:26:11Z

This seems to break tests on my Windows box: http://45.33.8.238/win/91548/step_6.txt

This is the failed test:
c:\src\llvm-project\out\gn\bin\clang.exe -Ofast -O2 -### -Werror C:\src\llvm-project\clang\test\Driver\Ofast.c 2>&1 | c:\src\llvm-project\out\gn\bin\filecheck.exe -check-prefix=CHECK-OFAST-O2    --check-prefix=CHECK-OFAST-O2-ALIASING-MSVC  C:\src\llvm-project\clang\test\Driver\Ofast.c
# executed command: 'c:\src\llvm-project\out\gn\bin\clang.exe' -Ofast -O2 '-###' -Werror 'C:\src\llvm-project\clang\test\Driver\Ofast.c'
# note: command had no output on stdout or stderr
# error: command failed with exit status: 1
# executed command: 'c:\src\llvm-project\out\gn\bin\filecheck.exe' -check-prefix=CHECK-OFAST-O2 --check-prefix=CHECK-OFAST-O2-ALIASING-MSVC 'C:\src\llvm-project\clang\test\Driver\Ofast.c'
Somehow clang.exe -Ofast -O2 -### -Werror C:\src\llvm-project\clang\test\Driver\Ofast.c 2>&1 is not producing any output for you, despite -### being present. I'm not sure why, but all windows buildbots seem to be happy with this patch. Can you investigate further?

CC @AaronBallman @MaskRay

FWIW, I'm not able to reproduce that behavior on my Windows machine.

MaskRay · 2024-07-24T15:34:26Z

This seems to break tests on my Windows box: http://45.33.8.238/win/91548/step_6.txt

This is the failed test:
c:\src\llvm-project\out\gn\bin\clang.exe -Ofast -O2 -### -Werror C:\src\llvm-project\clang\test\Driver\Ofast.c 2>&1 | c:\src\llvm-project\out\gn\bin\filecheck.exe -check-prefix=CHECK-OFAST-O2    --check-prefix=CHECK-OFAST-O2-ALIASING-MSVC  C:\src\llvm-project\clang\test\Driver\Ofast.c
# executed command: 'c:\src\llvm-project\out\gn\bin\clang.exe' -Ofast -O2 '-###' -Werror 'C:\src\llvm-project\clang\test\Driver\Ofast.c'
# note: command had no output on stdout or stderr
# error: command failed with exit status: 1
# executed command: 'c:\src\llvm-project\out\gn\bin\filecheck.exe' -check-prefix=CHECK-OFAST-O2 --check-prefix=CHECK-OFAST-O2-ALIASING-MSVC 'C:\src\llvm-project\clang\test\Driver\Ofast.c'
Somehow clang.exe -Ofast -O2 -### -Werror C:\src\llvm-project\clang\test\Driver\Ofast.c 2>&1 is not producing any output for you, despite -### being present. I'm not sure why, but all windows buildbots seem to be happy with this patch. Can you investigate further?

CC @AaronBallman @MaskRay

One possibility is that -Wno-msvc-not-found is relevant: c3a5087 .
(I think the -Wno-msvc-not-found behavior should be reworked, either removing the driver diagnostic , or delaying the diagnostic after everything else has been reported.)

The same "diagnostics are suppressed" issue happened recently on two bots that @dyung maintains, but I do not know whether theirs are Windows bots.

dyung · 2024-07-24T18:27:21Z

This seems to break tests on my Windows box: http://45.33.8.238/win/91548/step_6.txt

This is the failed test:
c:\src\llvm-project\out\gn\bin\clang.exe -Ofast -O2 -### -Werror C:\src\llvm-project\clang\test\Driver\Ofast.c 2>&1 | c:\src\llvm-project\out\gn\bin\filecheck.exe -check-prefix=CHECK-OFAST-O2    --check-prefix=CHECK-OFAST-O2-ALIASING-MSVC  C:\src\llvm-project\clang\test\Driver\Ofast.c
# executed command: 'c:\src\llvm-project\out\gn\bin\clang.exe' -Ofast -O2 '-###' -Werror 'C:\src\llvm-project\clang\test\Driver\Ofast.c'
# note: command had no output on stdout or stderr
# error: command failed with exit status: 1
# executed command: 'c:\src\llvm-project\out\gn\bin\filecheck.exe' -check-prefix=CHECK-OFAST-O2 --check-prefix=CHECK-OFAST-O2-ALIASING-MSVC 'C:\src\llvm-project\clang\test\Driver\Ofast.c'
Somehow clang.exe -Ofast -O2 -### -Werror C:\src\llvm-project\clang\test\Driver\Ofast.c 2>&1 is not producing any output for you, despite -### being present. I'm not sure why, but all windows buildbots seem to be happy with this patch. Can you investigate further?
CC @AaronBallman @MaskRay
One possibility is that -Wno-msvc-not-found is relevant: c3a5087 . (I think the -Wno-msvc-not-found behavior should be reworked, either removing the driver diagnostic , or delaying the diagnostic after everything else has been reported.)

The same "diagnostics are suppressed" issue happened recently on two bots that @dyung maintains, but I do not know whether theirs are Windows bots.

For my bots, the bot description should tell you what the bot is running, but I only maintain two builders that run Windows at the moment, but they are the same configuration, only one is a release branch version of the builder, but they run on the same physical worker. Do you have a link to the failures?

AaronBallman · 2024-07-25T13:43:52Z

Could you add a flang driver test as well?

The RFC was specifically about -Ofast in Clang and not behavior in Flang, so what test behavior would you like to see? This patch currently deprecates for both Clang and Flang, is that acceptable?

I was asking for a test that checks for the warning in Flang. But if @Endilll cannot build flang, you can proceed with the patch. I will create a patch separately for Flang.

Just making sure this doesn't fall through the cracks now that we've branched. Is Flang in a good state? e.g., are flang folks okay with deprecating -Ofast, and if so, has the test coverage been added? Or does the driver for Flang need be updated to not deprecate -Ofast?

Summary: This patch implements consensus on the corresponding RFC documented here: https://discourse.llvm.org/t/rfc-deprecate-ofast/78687/72 Specifically, I added a deprecation warning for `-Ofast`, that suggests to use `-O3` or `-O3` with `-ffast-math`, and a new diagnostic group for aforementioned warning. Deprecation period is going to be lengthy, so I hope this PR can be merged in time for Clang 19. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251517

Summary: A follow-up for #98736 Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251383

nico · 2024-09-06T12:23:47Z

Somehow clang.exe -Ofast -O2 -### -Werror C:\src\llvm-project\clang\test\Driver\Ofast.c 2>&1 is not producing any output for you, despite -### being present. I'm not sure why, but all windows buildbots seem to be happy with this patch. Can you investigate further?

Finally coming back to this. The problem is that I run tests without vcvarsall.bat run, i.e. I don't have MSVC's tools on PATH. env -i bin/clang/exe -Ofast -O2 -### -Werrror clang/test/Driver/Ofast.c then prints "clang: error: unable to find a Visual Studio installation; try running Clang from a developer prompt [-Werror,-Wmsvc-not-found]". It looks like this is the only test that

a) schedules to run the linker and
b) uses -Werror

Just adding a -c on line 6 (so that the driver doesn't try to discover a linker binary) makes the test pass on that bot. I'm guessing adding that is fine? Probably also for the other run lines in the test?

(Alternatively, -fuse-ld=lld would likely do the trick too, but from what I can tell the test isn't actually trying to test linker-related things.)

nico · 2024-09-06T12:24:55Z

…passing Wno-msvc-not-found as suggested above would also make the test go. But -c seems generally nicer here anyways?

Without this, `-###` prints the linker invocation as well, which can lead to `-Wno-msvc-not-found` warnings on Windows bots that don't have MSVC on path, causing the test to fail. Since the test isn't trying to test linker-related things, just pass `-c`. See discussion on #98736.

nico · 2024-09-06T12:28:24Z

I made that change in 5cf3677.

'-Ofast' enables optimizations that are not standards compliant, which can produce unexpected results unless used carefully. When building with clang 19 and newer, it warns: clang: error: argument '-Ofast' is deprecated; use '-O3 -ffast-math' for the same behavior, or '-O3' to enable only conforming optimizations [-Werror,-Wdeprecated-ofast] Relevant discussion: * llvm/llvm-project#98736 * https://discourse.llvm.org/t/rfc-deprecate-ofast/78687/109 * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115990 Signed-off-by: Tom Hughes <[email protected]>

[clang] Add deprecation warning for -Ofast driver option

44b4a68

Endilll added the clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' label Jul 13, 2024

Endilll requested review from MaskRay, mizvekov, jyknight, cor3ntin and AaronBallman July 13, 2024 10:13

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 13, 2024

MaskRay reviewed Jul 13, 2024

View reviewed changes

clang/test/Driver/Ofast.c Outdated Show resolved Hide resolved

MaskRay approved these changes Jul 13, 2024

View reviewed changes

jyknight reviewed Jul 14, 2024

View reviewed changes

AaronBallman reviewed Jul 15, 2024

View reviewed changes

Address reviewer feedback

c9f9cde

MaskRay reviewed Jul 15, 2024

View reviewed changes

clang/test/Driver/Ofast.c Outdated Show resolved Hide resolved

Fix typo

de66e7a

cor3ntin approved these changes Jul 16, 2024

View reviewed changes

Use Aaron's wording and replace CHECK-NOT with -Werror

ab2f6f5

AaronBallman approved these changes Jul 16, 2024

View reviewed changes

jyknight approved these changes Jul 16, 2024

View reviewed changes

mizvekov approved these changes Jul 16, 2024

View reviewed changes

Mention deprecation in the command guide

110d3fe

shafik reviewed Jul 18, 2024

View reviewed changes

clang/include/clang/Driver/Options.td Outdated Show resolved Hide resolved

Apply Shafik's suggestion

ee3c23e

Endilll merged commit 2ef7cbf into llvm:main Jul 18, 2024
5 of 7 checks passed

Endilll deleted the deprecate-ofast branch July 18, 2024 07:49

sebpop reviewed Jul 18, 2024

View reviewed changes

clang/include/clang/Basic/DiagnosticDriverKinds.td Show resolved Hide resolved

Endilll added a commit that referenced this pull request Jul 19, 2024

[clang][NFC] Fix typo in -Ofast deprecation warning

5a45fed

A follow-up for #98736

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024

[clang][NFC] Fix typo in -Ofast deprecation warning

c9edf3e

Summary: A follow-up for #98736 Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251383

thughes mentioned this pull request Mar 21, 2025

compiler_flags: Change optimization_fast from '-Ofast' to '-O3' zephyrproject-rtos/zephyr#87495

Closed

[clang] Add deprecation warning for -Ofast driver option #98736

[clang] Add deprecation warning for -Ofast driver option #98736

Uh oh!

Conversation

Endilll commented Jul 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jyknight left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

tschuett commented Jul 16, 2024

Uh oh!

Endilll commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tschuett commented Jul 16, 2024

Uh oh!

cor3ntin commented Jul 16, 2024

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Endilll commented Jul 16, 2024

Uh oh!

jyknight left a comment

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan commented Jul 16, 2024

Uh oh!

Endilll commented Jul 16, 2024

Uh oh!

kiranchandramohan commented Jul 16, 2024 via email

Uh oh!

AaronBallman commented Jul 16, 2024

Uh oh!

Uh oh!

Endilll commented Jul 18, 2024

Uh oh!

Uh oh!

sjoerdmeijer commented Jul 18, 2024

Uh oh!

Uh oh!

nico commented Jul 22, 2024

Uh oh!

Endilll commented Jul 23, 2024

Uh oh!

AaronBallman commented Jul 23, 2024

Uh oh!

MaskRay commented Jul 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

[clang] Add deprecation warning for `-Ofast` driver option #98736

[clang] Add deprecation warning for `-Ofast` driver option #98736

Endilll commented Jul 13, 2024 •

edited

Loading

llvmbot commented Jul 13, 2024 •

edited

Loading

Endilll commented Jul 16, 2024 •

edited

Loading

MaskRay commented Jul 24, 2024 •

edited

Loading