Workaround for MSVC ARM64 build performance regression

[lxbndr]

MSVC has a major performance regression observed when targeting ARM64 since v19.32 (VS 17.2.0). cl.exe spends a lot of time on compiling StandardLibrary.cpp and CGBuiltin.cpp, and total build duration rises extremely. This makes builds stagnate even on a real hardware, but VM-based builds (like building on cloud agents from GitHub Actions and Azure Pipelines) are experiencing most damage as they also performance- and time-limited.

The issue appears to be related to some optimizations applied in /O2 mode. It is reported on Developer Community. While the investigation is in progress, we could apply a workaround to improve build time. /O2 actually enables a set of optimizations, and only one of them does all slowdown. The idea is to disable optimizations, and then apply all but one back, effectively excluding the problematic option from the set.

This patch alters the CMake configuration for aforementioned files. Changes are limited to:

• non-debug builds
• MSVC of the specific version
• target arch (ARM64).

[tru]

Thanks for the PR, I think we at least need to make sure that we don't erase flags that have been set by the user manually.

Another option would just to say that this configuration ARM64 + MSVC of this version is not a valid configuration for MSVC and add it to ProblematicConfigurations here: https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/CheckProblematicConfigurations.cmake

[tru]

You are setting these settings for all configurations and erasing the other flags set for these files. I don't think that's the right solution. I would rather you grab the current compile_flags and then substitute the optimization flags only.

[lxbndr]

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, /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?

[tru]

Yeah I think this is what we want. We don't want to assume what the current flags are. Does it only happen in Release and RelWithDebInfo configurations? In that case you can maybe use generator expressions to only happen for those configurations?

Btw I noticed that COMPILE_FLAGS is deprecated and COMPILE_OPTIONS should be used instead, so maybe look into that as well?

[tru]

I think this comment also should link to the issue on the Microsoft side so that we can know to remove this later.

[lxbndr]

I was not sure if the link is acceptable to mention there. Will do, thanks!

[omjavaid]

Hi Have you compared build performance between cl and clang-cl? If only cl.exe is taking a lot of time while clang-cl doesnt then may be we should make this change specific to cl.exe.

[tru]

I suspect maybe the MSVC_VERSION check might exclude clang-cl, but we should make sure, because @omjavaid is correct, we shouldn't apply this to clang-cl unless it shows the same problems.

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Changes

MSVC has a major performance regression observed when targeting ARM64 since v19.32 (VS 17.2.0). cl.exe spends a lot of time on compiling StandardLibrary.cpp and CGBuiltin.cpp, and total build duration rises extremely. This makes builds stagnate even on a real hardware, but VM-based builds (like building on cloud agents from GitHub Actions and Azure Pipelines) are experiencing most damage as they also performance- and time-limited.

The issue appears to be related to some optimizations applied in /O2 mode. It is reported on Developer Community. While the investigation is in progress, we could apply a workaround to improve build time. /O2 actually enables a set of optimizations, and only one of them does all slowdown. The idea is to disable optimizations, and then apply all but one back, effectively excluding the problematic option from the set.

This patch alters the CMake configuration for aforementioned files. Changes are limited to:

• non-debug builds
• MSVC of the specific version
• target arch (ARM64).

---

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

2 Files Affected:

• (modified) clang/lib/CodeGen/CMakeLists.txt (+20)
• (modified) clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt (+20)

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
 

[lxbndr]

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

[tru]

LGTM

[lxbndr]

@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.

[xgupta]

@tru Is it fine to commit this?

[tru]

Yep. Want me to do it or do you have access now?

[xgupta]

Yes, I have the access.