[clang] Workaround for MSVC ARM64 build performance regression (PR #65215)
https://github.com/lxbndr updated
https://github.com/llvm/llvm-project/pull/65215
>From c97d9a28ef8cdb232670a960a46bc30b269983f7 Mon Sep 17 00:00:00 2001
From: Alexander Smarus
Date: Tue, 19 Sep 2023 00:34:54 +0300
Subject: [PATCH] Workaround for MSVC ARM64 build performance regression
---
clang/lib/CodeGen/CMakeLists.txt | 20 +++
.../Tooling/Inclusions/Stdlib/CMakeLists.txt | 20 +++
2 files changed, 40 insertions(+)
diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index 1debeb6d9cce9e0..2dd93a57f4e22d2 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -30,6 +30,26 @@ set(LLVM_LINK_COMPONENTS
TransformUtils
)
+# Workaround for MSVC ARM64 performance regression:
+#
https://developercommunity.visualstudio.com/t/Compiling-a-specific-code-for-ARM64-with/10444970
+# Since /O1 and /O2 represent a set of optimizations,
+# our goal is to disable the /Og flag while retaining the other optimizations
from the /O1|/O2 set
+if(MSVC AND NOT CMAKE_CXX_COMPILER_ID MATCHES Clang
+AND MSVC_VERSION VERSION_GREATER_EQUAL 1932
+AND CMAKE_SYSTEM_PROCESSOR MATCHES "ARM64")
+
+ string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
+ string(REGEX MATCHALL "/[Oo][12]" opt_flags "${CMAKE_CXX_FLAGS}
${CMAKE_CXX_FLAGS_${uppercase_CMAKE_BUILD_TYPE}}")
+ if (opt_flags)
+if(opt_flags MATCHES "1$")
+ set(opt_flags "/Od;/Os;/Oy;/Ob2;/GF;/Gy")
+elseif (opt_flags MATCHES "2$")
+ set(opt_flags "/Od;/Oi;/Ot;/Oy;/Ob2;/GF;/Gy")
+endif()
+set_source_files_properties(CGBuiltin.cpp PROPERTIES COMPILE_OPTIONS
"${opt_flags}")
+ endif()
+endif()
+
add_clang_library(clangCodeGen
ABIInfo.cpp
ABIInfoImpl.cpp
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
b/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
index 0f52c3590ac9af1..ed323ab3528b104 100644
--- a/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
+++ b/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
@@ -1,3 +1,23 @@
+# Workaround for MSVC ARM64 performance regression:
+#
https://developercommunity.visualstudio.com/t/Compiling-a-specific-code-for-ARM64-with/10444970
+# Since /O1 and /O2 represent a set of optimizations,
+# our goal is to disable the /Og flag while retaining the other optimizations
from the /O1|/O2 set
+if(MSVC AND NOT CMAKE_CXX_COMPILER_ID MATCHES Clang
+AND MSVC_VERSION VERSION_GREATER_EQUAL 1932
+AND CMAKE_SYSTEM_PROCESSOR MATCHES "ARM64")
+
+ string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
+ string(REGEX MATCHALL "/[Oo][12]" opt_flags "${CMAKE_CXX_FLAGS}
${CMAKE_CXX_FLAGS_${uppercase_CMAKE_BUILD_TYPE}}")
+ if (opt_flags)
+if(opt_flags MATCHES "1$")
+ set(opt_flags "/Od;/Os;/Oy;/Ob2;/GF;/Gy")
+elseif (opt_flags MATCHES "2$")
+ set(opt_flags "/Od;/Oi;/Ot;/Oy;/Ob2;/GF;/Gy")
+endif()
+set_source_files_properties(StandardLibrary.cpp PROPERTIES COMPILE_OPTIONS
"${opt_flags}")
+ endif()
+endif()
+
add_clang_library(clangToolingInclusionsStdlib
StandardLibrary.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Workaround for MSVC ARM64 build performance regression (PR #65215)
lxbndr wrote: A short summary of changes: - added link to the issue - narrowed checks to aim cl.exe only (clang-cl.exe doesn't need this) - changed approach a bit. Now we're looking for /O2 and /O1 flags in the general flag set for the current configuration. Last effective found flag is suppressed and replaced with "same-minus-/Og" equivalent for corresponding O variant (either size or speed). I believe this is the most careful way to adjust the build options and avoid conflicts with user flags. Basically, if you have /O_n_, you will get same optimizations except problematic one. cc @tru @omjavaid https://github.com/llvm/llvm-project/pull/65215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Workaround for MSVC ARM64 build performance regression (PR #65215)
lxbndr wrote: @tru I apologize for any inconvenience, but should I rebase this to the latest master, or something else to unblock landing? I am not sure if I see any suitable rule in the contribution guide. Just in case it occasionally turns out that the blocker is me. https://github.com/llvm/llvm-project/pull/65215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Workaround for MSVC ARM64 build performance regression (PR #65215)
https://github.com/lxbndr review_requested https://github.com/llvm/llvm-project/pull/65215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Workaround for MSVC ARM64 build performance regression (PR #65215)
https://github.com/lxbndr opened https://github.com/llvm/llvm-project/pull/65215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Workaround for MSVC ARM64 build performance regression (PR #65215)
@@ -30,6 +30,15 @@ set(LLVM_LINK_COMPONENTS TransformUtils ) +# Workaround for MSVC ARM64 performance regression: disable all optimizations (/Od) +# and then enable back all /O2 options except one. +if(NOT CMAKE_BUILD_TYPE MATCHES Debug +AND MSVC +AND MSVC_VERSION VERSION_GREATER_EQUAL 1932 +AND CMAKE_SYSTEM_PROCESSOR MATCHES "ARM64") + set_source_files_properties(CGBuiltin.cpp PROPERTIES COMPILE_FLAGS "/Od /Gw /Oi /Oy /Gy /Ob2 /Ot /GF") lxbndr wrote: Thanks for the review! I think I have to elaborate a little (and I apologize for not making this clear enough in the PR description). We're not going to just throw a bunch of optimization flags, of course. The flag I actually intend to alter is `/O2`. According to the [reference](https://learn.microsoft.com/en-us/cpp/build/reference/o1-o2-minimize-size-maximize-speed?view=msvc-170), `/O2` is the equivalent of `/Og /Oi /Ot /Oy /Ob2 /GF /Gy` (I even see this in the compiler frontend invocation). I want to suppress the `/Og` and leave others in effect. This is done by adding `/Od` first (which cancels all `/O2` flags), and then adding "O2-minus-Og" flags back. Probably, the `/Gw` flag is the only unnecessary flag here, as it is not part of the `/O...` set (and it is not affected by `/Od` either). It is also worth to mention that `set_source_files_properties` does not override flag set. It appends specified flags to other configured flags. That's why I had to use `/Od` to cancel already existing `/O2`. I fully understand the intention to not break existing configuration. And I think I see how this can be improved. Instead of assuming all non-debug builds have optimizations enabled, we should seek for `/O2`/`/O1` flags directly, and adjust resulting flag set according to each case. @tru WDYT? Does this sound better? https://github.com/llvm/llvm-project/pull/65215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Workaround for MSVC ARM64 build performance regression (PR #65215)
@@ -30,6 +30,15 @@ set(LLVM_LINK_COMPONENTS TransformUtils ) +# Workaround for MSVC ARM64 performance regression: disable all optimizations (/Od) lxbndr wrote: I was not sure if the link is acceptable to mention there. Will do, thanks! https://github.com/llvm/llvm-project/pull/65215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Adjust MSVC version range for ARM64 build performance regression (PR #90731)
https://github.com/lxbndr created
https://github.com/llvm/llvm-project/pull/90731
This is follow up for #65215
Mentioned regression was fixed in MSVC 19.39 (VS 17.9.0), so it makes sense to
not apply fix for that (and newer) compiler versions.
Same as original change, this patch is narrowly scoped to not affect any other
compiler.
>From ff8c63595aca6aca603deb109c3684e7df59e61b Mon Sep 17 00:00:00 2001
From: Alexander Smarus
Date: Wed, 1 May 2024 16:43:04 +0300
Subject: [PATCH] Adjust MSVC version range for ARM64 build performance
regression
---
clang/lib/CodeGen/CMakeLists.txt | 1 +
clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt | 1 +
2 files changed, 2 insertions(+)
diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index 52216d93a302bb..760fc540f7fa8e 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -39,6 +39,7 @@ set(LLVM_LINK_COMPONENTS
# our goal is to disable the /Og flag while retaining the other optimizations
from the /O1|/O2 set
if(MSVC AND NOT CMAKE_CXX_COMPILER_ID MATCHES Clang
AND MSVC_VERSION VERSION_GREATER_EQUAL 1932
+AND MSVC_VERSION VERSION_LESS 1939
AND CMAKE_SYSTEM_PROCESSOR MATCHES "ARM64")
string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
b/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
index ed323ab3528b10..4d01a0850a074b 100644
--- a/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
+++ b/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
@@ -4,6 +4,7 @@
# our goal is to disable the /Og flag while retaining the other optimizations
from the /O1|/O2 set
if(MSVC AND NOT CMAKE_CXX_COMPILER_ID MATCHES Clang
AND MSVC_VERSION VERSION_GREATER_EQUAL 1932
+AND MSVC_VERSION VERSION_LESS 1939
AND CMAKE_SYSTEM_PROCESSOR MATCHES "ARM64")
string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Adjust MSVC version range for ARM64 build performance regression (PR #90731)
https://github.com/lxbndr updated
https://github.com/llvm/llvm-project/pull/90731
>From 52fe3d377e5f6a0388f102b82283529253d4ab4c Mon Sep 17 00:00:00 2001
From: Alexander Smarus
Date: Wed, 1 May 2024 16:43:04 +0300
Subject: [PATCH] Adjust MSVC version range for ARM64 build performance
regression
---
clang/lib/CodeGen/CMakeLists.txt | 1 +
clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt | 1 +
2 files changed, 2 insertions(+)
diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index 8dd9d8b54c25f..2a179deddcc31 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -39,6 +39,7 @@ set(LLVM_LINK_COMPONENTS
# our goal is to disable the /Og flag while retaining the other optimizations
from the /O1|/O2 set
if(MSVC AND NOT CMAKE_CXX_COMPILER_ID MATCHES Clang
AND MSVC_VERSION VERSION_GREATER_EQUAL 1932
+AND MSVC_VERSION VERSION_LESS 1939
AND CMAKE_SYSTEM_PROCESSOR MATCHES "ARM64")
string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
b/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
index ed323ab3528b1..4d01a0850a074 100644
--- a/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
+++ b/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
@@ -4,6 +4,7 @@
# our goal is to disable the /Og flag while retaining the other optimizations
from the /O1|/O2 set
if(MSVC AND NOT CMAKE_CXX_COMPILER_ID MATCHES Clang
AND MSVC_VERSION VERSION_GREATER_EQUAL 1932
+AND MSVC_VERSION VERSION_LESS 1939
AND CMAKE_SYSTEM_PROCESSOR MATCHES "ARM64")
string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Adjust MSVC version range for ARM64 build performance regression (PR #90731)
lxbndr wrote: Rebased on latest main. Let's see what CI say. https://github.com/llvm/llvm-project/pull/90731 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
