From 698e9f71cd72ed6176938ff093b40e57f92cd681 Mon Sep 17 00:00:00 2001
From: Yi Zhang <zhanyi@microsoft.com>
Date: Wed, 19 Apr 2023 00:31:35 +0800
Subject: [PATCH] Improve cache hit rate in windows build (#15538)

### Description
1.  Update /Zi to /Z7 in abseil project while using cache
2.  Skip target_precompile_headers while using cache


### Motivation and Context
There're about 1/4 uncacheable calls in Windows GPU compilation with
cache.
```
Uncacheable calls:                   441 / 1641 (26.87%)
  Could not use precompiled header:  361 /  441 (81.86%)
  Preprocessing failed:                1 /  441 ( 0.23%)
  Unsupported compiler option:        79 /  441 (17.91%)
```

https://dev.azure.com/onnxruntime/onnxruntime/_build/results?buildId=961916&view=logs&j=5076e696-f193-5f12-2d8a-703dda41a79b&t=9b927034-e3ef-5e25-c6df-387bc37acd63&l=21

The root cause of `Unsupported compiler option` is that /Zi in Abseil
isn't updated to /Z7.
The root cause of `Could not use precompiled header` is the
`target_precompile_headers` creates cmake_pch.pch every time and it's
hash value is changed too.

### Result
It could reduce compilation time by another 20%.
For example:
It took 16m43 in CUDA training compilation on Windows.
It takes 13m32 after the change.

https://dev.azure.com/onnxruntime/onnxruntime/_build/results?buildId=964002&view=logs&s=959c6b43-5937-53e5-5f36-e53cb0249117


### N.B.
In winml project, it's using own target_precompile**d**_header
https://github.com/microsoft/onnxruntime/blob/main/cmake/precompiled_header.cmake.
Just let it be.
---
 cmake/CMakeLists.txt                  | 23 ++++++++++-------------
 cmake/external/helper_functions.cmake | 12 ------------
 cmake/onnxruntime_providers.cmake     | 10 ++++++----
 3 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/cmake/CMakeLists.txt b/cmake/CMakeLists.txt
index b1135bdad6..4af668c3d2 100644
--- a/cmake/CMakeLists.txt
+++ b/cmake/CMakeLists.txt
@@ -12,6 +12,11 @@ cmake_policy(SET CMP0117 NEW)
 # Don't let cmake set a default value for CMAKE_CUDA_ARCHITECTURES
 cmake_policy(SET CMP0104 OLD)
 
+# Enable Hot Reload for MSVC compilers if supported.
+if (POLICY CMP0141)
+  cmake_policy(SET CMP0141 NEW)
+endif()
+
 # Project
 project(onnxruntime C CXX ASM)
 
@@ -332,6 +337,11 @@ function(set_msvc_c_cpp_compiler_warning_level warning_level)
   endif()
 endfunction()
 
+
+if(MSVC_Z7_OVERRIDE)
+  set(CMAKE_MSVC_DEBUG_INFORMATION_FORMAT "$<$<CONFIG:Debug,RelWithDebInfo>:Embedded>")
+endif(MSVC_Z7_OVERRIDE)
+
 # set default MSVC warning level to 3 for external dependencies
 set_msvc_c_cpp_compiler_warning_level(3)
 
@@ -722,19 +732,6 @@ if (onnxruntime_ENABLE_LAZY_TENSOR)
   # cleaner.
 endif()
 
-if(MSVC)
-  # Replace /Zi and /ZI with /Z7
-  if(MSVC_Z7_OVERRIDE)
-    foreach(flag_var
-      CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG  CMAKE_C_FLAGS_RELWITHDEBINFO
-      CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG  CMAKE_CXX_FLAGS_RELWITHDEBINFO)
-      if(${flag_var} MATCHES "/Z[iI]")
-        string(REGEX REPLACE "/Z[iI]" "/Z7" ${flag_var} "${${flag_var}}")
-      endif(${flag_var} MATCHES "/Z[iI]")
-    endforeach(flag_var)
-  endif(MSVC_Z7_OVERRIDE)
-endif()
-
 function(onnxruntime_set_compile_flags target_name)
     if (CPUINFO_SUPPORTED)
       onnxruntime_add_include_to_target(${target_name} cpuinfo::cpuinfo)
diff --git a/cmake/external/helper_functions.cmake b/cmake/external/helper_functions.cmake
index 88b46890b7..768e807b40 100644
--- a/cmake/external/helper_functions.cmake
+++ b/cmake/external/helper_functions.cmake
@@ -176,18 +176,6 @@ macro(onnxruntime_fetchcontent_makeavailable)
               set_target_properties(${subdir_target} PROPERTIES COMPILE_WARNING_AS_ERROR OFF)
             endif()
           endforeach()
-          if(MSVC)
-            # Replace /Zi and /ZI with /Z7
-            if(MSVC_Z7_OVERRIDE)
-              foreach(flag_var
-                CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG  CMAKE_C_FLAGS_RELWITHDEBINFO
-                CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG  CMAKE_CXX_FLAGS_RELWITHDEBINFO)
-                if(${flag_var} MATCHES "/Z[iI]")
-                  string(REGEX REPLACE "/Z[iI]" "/Z7" ${flag_var} "${${flag_var}}")
-                endif(${flag_var} MATCHES "/Z[iI]")
-              endforeach(flag_var)
-            endif(MSVC_Z7_OVERRIDE)
-        endif()
       endif()
 
       unset(__cmake_srcdir)
diff --git a/cmake/onnxruntime_providers.cmake b/cmake/onnxruntime_providers.cmake
index de46811f22..ad46a0e07e 100644
--- a/cmake/onnxruntime_providers.cmake
+++ b/cmake/onnxruntime_providers.cmake
@@ -529,10 +529,12 @@ if (onnxruntime_USE_CUDA)
 
   if (WIN32)
     # *.cu cannot use PCH
-    target_precompile_headers(onnxruntime_providers_cuda PUBLIC
-      "${ONNXRUNTIME_ROOT}/core/providers/cuda/cuda_pch.h"
-      "${ONNXRUNTIME_ROOT}/core/providers/cuda/cuda_pch.cc"
-    )
+    if (NOT onnxruntime_BUILD_CACHE)
+      target_precompile_headers(onnxruntime_providers_cuda PUBLIC
+        "${ONNXRUNTIME_ROOT}/core/providers/cuda/cuda_pch.h"
+        "${ONNXRUNTIME_ROOT}/core/providers/cuda/cuda_pch.cc"
+      )
+    endif()
 
     # minimize the Windows includes.
     # this avoids an issue with CUDA 11.6 where 'small' is defined in the windows and cuda headers.