-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang-cl] Map /Ot to -O3 instead of -O2 #95406
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
Conversation
/Ot (which is also implied by /O2) is supposed to optimize for maximum speed, so -O3 seems like a better match.
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Hans (zmodem) Changes/Ot (which is also implied by /O2) is supposed to optimize for maximum speed, so -O3 seems like a better match. Full diff: https://github.com/llvm/llvm-project/pull/95406.diff 5 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8c2f737836a9d..68355dbb5861b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -921,6 +921,10 @@ Android Support
Windows Support
^^^^^^^^^^^^^^^
+- The clang-cl ``/Ot`` compiler option ("optimize for speed", also implied by
+ ``/O2``) now maps to clang's ``-O3`` optimizataztion level instead of ``-O2``.
+ Users who prefer the old behavior can use ``clang-cl /Ot /clang:-O2 ...``.
+
- Clang-cl now supports function targets with intrinsic headers. This allows
for runtime feature detection of intrinsics. Previously under clang-cl
``immintrin.h`` and similar intrinsic headers would only include the intrinsics
diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index f954857b0235a..ee30e4eff9ea0 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -4636,8 +4636,8 @@ Execute ``clang-cl /?`` to see a list of supported options:
/Og No effect
/Oi- Disable use of builtin functions
/Oi Enable use of builtin functions
- /Os Optimize for size
- /Ot Optimize for speed
+ /Os Optimize for size (like clang -Os)
+ /Ot Optimize for speed (like clang -O3)
/Ox Deprecated (same as /Og /Oi /Ot /Oy /Ob2); use /O2 instead
/Oy- Disable frame pointer omission (x86 only, default)
/Oy Enable frame pointer omission (x86 only)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 880221176027e..1620b12d55ed9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8279,9 +8279,9 @@ def : CLFlag<"Oi">, Alias<_SLASH_O>, AliasArgs<["i"]>,
def : CLFlag<"Oi-">, Alias<_SLASH_O>, AliasArgs<["i-"]>,
HelpText<"Disable use of builtin functions">;
def : CLFlag<"Os">, Alias<_SLASH_O>, AliasArgs<["s"]>,
- HelpText<"Optimize for size">;
+ HelpText<"Optimize for size (like clang -Os)">;
def : CLFlag<"Ot">, Alias<_SLASH_O>, AliasArgs<["t"]>,
- HelpText<"Optimize for speed">;
+ HelpText<"Optimize for speed (like clang -O3)">;
def : CLFlag<"Ox">, Alias<_SLASH_O>, AliasArgs<["x"]>,
HelpText<"Deprecated (like /Og /Oi /Ot /Oy /Ob2); use /O2">;
def : CLFlag<"Oy">, Alias<_SLASH_O>, AliasArgs<["y"]>,
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index b7021d4b996dd..d03687208c5c6 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -861,7 +861,7 @@ static void TranslateOptArg(Arg *A, llvm::opt::DerivedArgList &DAL,
DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s");
} else if (OptChar == '2' || OptChar == 'x') {
DAL.AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin));
- DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "2");
+ DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3");
}
if (SupportsForcingFramePointer &&
!DAL.hasArgNoClaim(options::OPT_fno_omit_frame_pointer))
@@ -901,7 +901,7 @@ static void TranslateOptArg(Arg *A, llvm::opt::DerivedArgList &DAL,
DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s");
break;
case 't':
- DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "2");
+ DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3");
break;
case 'y': {
bool OmitFramePointer = true;
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 2c17459dde656..e77ec364170d1 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -189,12 +189,12 @@
// RUN: %clang_cl /Ot --target=i686-pc-windows-msvc -### -- %s 2>&1 | FileCheck -check-prefix=Ot %s
// RUN: %clang_cl /Ot --target=x86_64-pc-windows-msvc -### -- %s 2>&1 | FileCheck -check-prefix=Ot %s
// Ot: -mframe-pointer=none
-// Ot: -O2
+// Ot: -O3
// RUN: %clang_cl /Ox --target=i686-pc-windows-msvc -### -- %s 2>&1 | FileCheck -check-prefix=Ox %s
// RUN: %clang_cl /Ox --target=x86_64-pc-windows-msvc -### -- %s 2>&1 | FileCheck -check-prefix=Ox %s
// Ox: -mframe-pointer=none
-// Ox: -O2
+// Ox: -O3
// RUN: %clang_cl --target=i686-pc-win32 /O2sy- -### -- %s 2>&1 | FileCheck -check-prefix=PR24003 %s
// PR24003: -mframe-pointer=all
@@ -202,14 +202,14 @@
// RUN: %clang_cl --target=i686-pc-win32 -Werror -Wno-msvc-not-found /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_2 %s
// Oy_2: -mframe-pointer=all
-// Oy_2: -O2
+// Oy_2: -O3
// RUN: %clang_cl --target=aarch64-pc-windows-msvc -Werror -Wno-msvc-not-found /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_aarch64 %s
// Oy_aarch64: -mframe-pointer=non-leaf
-// Oy_aarch64: -O2
+// Oy_aarch64: -O3
// RUN: %clang_cl --target=i686-pc-win32 -Werror -Wno-msvc-not-found /O2 /O2 -### -- %s 2>&1 | FileCheck -check-prefix=O2O2 %s
-// O2O2: "-O2"
+// O2O2: "-O3"
// RUN: %clang_cl /Zs -Werror /Oy -- %s 2>&1
|
@AaronBallman as discussed on https://discourse.llvm.org/t/clang-cl-optimization-option/79554 Also @nico who IIRC looked at these flags in recent times. |
Can you add something like "use /O2 /clang:-O2 to restore the previous behavior" to the commit message, on case someone prefers that? Otherwise LG. |
Oh, you have that in the release notes already, even better. |
If we're trying to match MSVC, the amount of inlining MSVC does at /O2 is probably closer to what clang does at -O2 than -O3. Which is why it was mapped that way in 015ce0f, I think. clang's -O3 is really aggressive (which tends to look good in benchmarks, but less so in real software). I'm a bit worried we're going to get a wave of complaints about codesize in reaction to this. |
For reference, current MSVC has a flag /Ob3 to request more aggressive inlining. |
…14561-gecea8371-1 / 32dd3795bce8b347fda786529cf5e42a813e0b7d-2 : 3cf924b934322fd7b514600a7dc84fc517515346-1 https://chromium.googlesource.com/external/github.com/llvm/llvm-project/+log/084e2b53..ecea8371 https://chromium.googlesource.com/external/github.com/rust-lang/rust/+log/32dd3795bce8..3cf924b93432 Also update clang-cl flags to pass "/clang:-O2" after /O2 after llvm/llvm-project#95406 Ran: ./tools/clang/scripts/upload_revision.py ecea8371ff03c15fb3dc27ee4108b98335fd2d63 tools/clang/scripts/sync_deps.py tools/rust/gnrt_stdlib.py third_party/abseil-cpp/generate_def_files.py Bug: 340924169 Change-Id: Ie3680f33261952f66d972398656661090c47726a Disable-Rts: True Binary-Size: Inlining heuristic change, see crbug.com/347984812. Cq-Include-Trybots: chromium/try:chromeos-amd64-generic-cfi-thin-lto-rel Cq-Include-Trybots: chromium/try:dawn-win10-x86-deps-rel Cq-Include-Trybots: chromium/try:lacros-arm64-generic-rel Cq-Include-Trybots: chromium/try:linux-chromeos-dbg Cq-Include-Trybots: chromium/try:linux_chromium_cfi_rel_ng Cq-Include-Trybots: chromium/try:linux_chromium_chromeos_msan_rel_ng Cq-Include-Trybots: chromium/try:linux_chromium_msan_rel_ng Cq-Include-Trybots: chromium/try:mac11-arm64-rel,mac_chromium_asan_rel_ng Cq-Include-Trybots: chromium/try:ios-catalyst,win-asan,android-official Cq-Include-Trybots: chromium/try:fuchsia-arm64-cast-receiver-rel Cq-Include-Trybots: chromium/try:mac-official,linux-official Cq-Include-Trybots: chromium/try:win-official,win32-official Cq-Include-Trybots: chromium/try:win-arm64-rel Cq-Include-Trybots: chromium/try:linux-swangle-try-x64,win-swangle-try-x86 Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-arm64-dbg Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-arm64-rel Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-x86-dbg Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-x86-rel Cq-Include-Trybots: chromium/try:android-cronet-riscv64-dbg Cq-Include-Trybots: chromium/try:android-cronet-riscv64-rel Cq-Include-Trybots: chrome/try:iphone-device,ipad-device Cq-Include-Trybots: chrome/try:linux-chromeos-chrome Cq-Include-Trybots: chrome/try:win-chrome,win64-chrome,linux-chrome,mac-chrome Cq-Include-Trybots: chrome/try:linux-pgo,mac-pgo,win32-pgo,win64-pgo Cq-Include-Trybots: chromium/try:android-rust-arm32-rel Cq-Include-Trybots: chromium/try:android-rust-arm64-dbg Cq-Include-Trybots: chromium/try:android-rust-arm64-rel Cq-Include-Trybots: chromium/try:linux-rust-x64-dbg Cq-Include-Trybots: chromium/try:linux-rust-x64-rel Cq-Include-Trybots: chromium/try:mac-rust-x64-dbg Cq-Include-Trybots: chromium/try:win-rust-x64-dbg Cq-Include-Trybots: chromium/try:win-rust-x64-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5633975 Reviewed-by: Nico Weber <[email protected]> Commit-Queue: Hans Wennborg <[email protected]> Cr-Commit-Position: refs/heads/main@{#1317359} CrOS-Libchrome-Original-Commit: 65d491f36bac46186a87e0cc890bb810dc66c4f1
…14561-gecea8371-1 / 32dd3795bce8b347fda786529cf5e42a813e0b7d-2 : 3cf924b934322fd7b514600a7dc84fc517515346-1 https://chromium.googlesource.com/external/github.com/llvm/llvm-project/+log/084e2b53..ecea8371 https://chromium.googlesource.com/external/github.com/rust-lang/rust/+log/32dd3795bce8..3cf924b93432 Also update clang-cl flags to pass "/clang:-O2" after /O2 after llvm/llvm-project#95406 Ran: ./tools/clang/scripts/upload_revision.py ecea8371ff03c15fb3dc27ee4108b98335fd2d63 tools/clang/scripts/sync_deps.py tools/rust/gnrt_stdlib.py third_party/abseil-cpp/generate_def_files.py Bug: 340924169 Change-Id: Ie3680f33261952f66d972398656661090c47726a Disable-Rts: True Binary-Size: Inlining heuristic change, see crbug.com/347984812. Cq-Include-Trybots: chromium/try:chromeos-amd64-generic-cfi-thin-lto-rel Cq-Include-Trybots: chromium/try:dawn-win10-x86-deps-rel Cq-Include-Trybots: chromium/try:lacros-arm64-generic-rel Cq-Include-Trybots: chromium/try:linux-chromeos-dbg Cq-Include-Trybots: chromium/try:linux_chromium_cfi_rel_ng Cq-Include-Trybots: chromium/try:linux_chromium_chromeos_msan_rel_ng Cq-Include-Trybots: chromium/try:linux_chromium_msan_rel_ng Cq-Include-Trybots: chromium/try:mac11-arm64-rel,mac_chromium_asan_rel_ng Cq-Include-Trybots: chromium/try:ios-catalyst,win-asan,android-official Cq-Include-Trybots: chromium/try:fuchsia-arm64-cast-receiver-rel Cq-Include-Trybots: chromium/try:mac-official,linux-official Cq-Include-Trybots: chromium/try:win-official,win32-official Cq-Include-Trybots: chromium/try:win-arm64-rel Cq-Include-Trybots: chromium/try:linux-swangle-try-x64,win-swangle-try-x86 Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-arm64-dbg Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-arm64-rel Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-x86-dbg Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-x86-rel Cq-Include-Trybots: chromium/try:android-cronet-riscv64-dbg Cq-Include-Trybots: chromium/try:android-cronet-riscv64-rel Cq-Include-Trybots: chrome/try:iphone-device,ipad-device Cq-Include-Trybots: chrome/try:linux-chromeos-chrome Cq-Include-Trybots: chrome/try:win-chrome,win64-chrome,linux-chrome,mac-chrome Cq-Include-Trybots: chrome/try:linux-pgo,mac-pgo,win32-pgo,win64-pgo Cq-Include-Trybots: chromium/try:android-rust-arm32-rel Cq-Include-Trybots: chromium/try:android-rust-arm64-dbg Cq-Include-Trybots: chromium/try:android-rust-arm64-rel Cq-Include-Trybots: chromium/try:linux-rust-x64-dbg Cq-Include-Trybots: chromium/try:linux-rust-x64-rel Cq-Include-Trybots: chromium/try:mac-rust-x64-dbg Cq-Include-Trybots: chromium/try:win-rust-x64-dbg Cq-Include-Trybots: chromium/try:win-rust-x64-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5633975 Reviewed-by: Nico Weber <[email protected]> Commit-Queue: Hans Wennborg <[email protected]> Cr-Commit-Position: refs/heads/main@{#1317359} NOKEYCHECK=True GitOrigin-RevId: 65d491f36bac46186a87e0cc890bb810dc66c4f1
/Ot (which is also implied by /O2) is supposed to optimize for maximum speed, so -O3 seems like a better match.
…14561-gecea8371-1 / 32dd3795bce8b347fda786529cf5e42a813e0b7d-2 : 3cf924b934322fd7b514600a7dc84fc517515346-1 https://chromium.googlesource.com/external/github.com/llvm/llvm-project/+log/084e2b53..ecea8371 https://chromium.googlesource.com/external/github.com/rust-lang/rust/+log/32dd3795bce8..3cf924b93432 Also update clang-cl flags to pass "/clang:-O2" after /O2 after llvm/llvm-project#95406 Ran: ./tools/clang/scripts/upload_revision.py ecea8371ff03c15fb3dc27ee4108b98335fd2d63 tools/clang/scripts/sync_deps.py tools/rust/gnrt_stdlib.py third_party/abseil-cpp/generate_def_files.py Bug: 340924169 Change-Id: Ie3680f33261952f66d972398656661090c47726a Disable-Rts: True Binary-Size: Inlining heuristic change, see crbug.com/347984812. Cq-Include-Trybots: chromium/try:chromeos-amd64-generic-cfi-thin-lto-rel Cq-Include-Trybots: chromium/try:dawn-win10-x86-deps-rel Cq-Include-Trybots: chromium/try:lacros-arm64-generic-rel Cq-Include-Trybots: chromium/try:linux-chromeos-dbg Cq-Include-Trybots: chromium/try:linux_chromium_cfi_rel_ng Cq-Include-Trybots: chromium/try:linux_chromium_chromeos_msan_rel_ng Cq-Include-Trybots: chromium/try:linux_chromium_msan_rel_ng Cq-Include-Trybots: chromium/try:mac11-arm64-rel,mac_chromium_asan_rel_ng Cq-Include-Trybots: chromium/try:ios-catalyst,win-asan,android-official Cq-Include-Trybots: chromium/try:fuchsia-arm64-cast-receiver-rel Cq-Include-Trybots: chromium/try:mac-official,linux-official Cq-Include-Trybots: chromium/try:win-official,win32-official Cq-Include-Trybots: chromium/try:win-arm64-rel Cq-Include-Trybots: chromium/try:linux-swangle-try-x64,win-swangle-try-x86 Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-arm64-dbg Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-arm64-rel Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-x86-dbg Cq-Include-Trybots: chromium/try:android-cronet-mainline-clang-x86-rel Cq-Include-Trybots: chromium/try:android-cronet-riscv64-dbg Cq-Include-Trybots: chromium/try:android-cronet-riscv64-rel Cq-Include-Trybots: chrome/try:iphone-device,ipad-device Cq-Include-Trybots: chrome/try:linux-chromeos-chrome Cq-Include-Trybots: chrome/try:win-chrome,win64-chrome,linux-chrome,mac-chrome Cq-Include-Trybots: chrome/try:linux-pgo,mac-pgo,win32-pgo,win64-pgo Cq-Include-Trybots: chromium/try:android-rust-arm32-rel Cq-Include-Trybots: chromium/try:android-rust-arm64-dbg Cq-Include-Trybots: chromium/try:android-rust-arm64-rel Cq-Include-Trybots: chromium/try:linux-rust-x64-dbg Cq-Include-Trybots: chromium/try:linux-rust-x64-rel Cq-Include-Trybots: chromium/try:mac-rust-x64-dbg Cq-Include-Trybots: chromium/try:win-rust-x64-dbg Cq-Include-Trybots: chromium/try:win-rust-x64-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5633975 Reviewed-by: Nico Weber <[email protected]> Commit-Queue: Hans Wennborg <[email protected]> Cr-Commit-Position: refs/heads/main@{#1317359} NOKEYCHECK=True GitOrigin-RevId: 65d491f36bac46186a87e0cc890bb810dc66c4f1
/Ot (which is also implied by /O2) is supposed to optimize for maximum speed, so -O3 seems like a better match.