[clang] be06761 - [clang][cmake] Fixes for PGO builds when invoking ninja twice (#92591)

[via cfe-commits]

Author: Tom Stellard
Date: 2024-06-10T13:01:58-07:00
New Revision: be067615453bab711fd4996c3c235ca996a306a1

URL: https://github.com/llvm/llvm-project/commit/be067615453bab711fd4996c3c235ca996a306a1
DIFF: https://github.com/llvm/llvm-project/commit/be067615453bab711fd4996c3c235ca996a306a1.diff

LOG: [clang][cmake] Fixes for PGO builds when invoking ninja twice (#92591)

This fixes two problems with the 2-stage PGO builds. The first problem
was that the stage2-instrumented and stage2 targets would not be built
on the second ninja invocation. For example:

This would work as expected.
$ ninja -v -C build stage2-instrumented-generate-profdata

Edit a file.
$ touch llvm/lib/Support/StringExtras.cpp

This would rebuild stage1 only and not build stage2-instrumented or
regenerate the profile data.
$ ninja -v -C build stage2-instrumented-generate-profdata

The second problem was that in some cases, the profile data would be
regenerated, but not all of the stage2 targets would be rebuilt. One
common scenario where this would happen is:

This would work as expected.
$ ninja -C build stage2-check-all

This would regenerate the profile data, but then only build the
targets that were needed for install, but weren't needed for
check-all.  This would cause errors like:
ld.lld: error: Function Import: link error: linking module flags
'ProfileSummary': IDs have conflicting values in ...
 This is because the executibles being built for the install target
used the new profile data, but they were linking with libraries that
used the old profile data.
$ ninja -C build stage2-install

With this change we can re-enable PGO for the release builds.

Added: 
    

Modified: 
    clang/CMakeLists.txt
    clang/cmake/caches/Release.cmake
    clang/utils/perf-training/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 2ac0bccb42f50..f6d96258cd135 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -848,23 +848,17 @@ if (CLANG_ENABLE_BOOTSTRAP)
     set(CLANG_BOOTSTRAP_TARGETS check-llvm check-clang check-all)
   endif()
   foreach(target ${CLANG_BOOTSTRAP_TARGETS})
-    # Install targets have side effects, so we always want to execute them.
-    # "install" is reserved by CMake and can't be used as a step name for
-    # ExternalProject_Add_Step, so we can match against "^install-" instead of
-    # "^install" to get a tighter match. CMake's installation scripts already
-    # skip up-to-date files, so there's no behavior change if you install to the
-    # same destination multiple times.
-    if(target MATCHES "^install-")
-      set(step_always ON)
-    else()
-      set(step_always OFF)
-    endif()
 
     ExternalProject_Add_Step(${NEXT_CLANG_STAGE} ${target}
       COMMAND ${CMAKE_COMMAND} --build <BINARY_DIR> --target ${target}
       COMMENT "Performing ${target} for '${NEXT_CLANG_STAGE}'"
       DEPENDEES configure
-      ALWAYS ${step_always}
+      # We need to set ALWAYS to ON here, otherwise these targets won't be
+      # built on a second invocation of ninja.  The targets have their own
+      # logic to determine if they should build or not so setting ALWAYS ON
+      # here does not mean the targets will always rebuild it just means that
+      # they will check their dependenices and see if they need to be built.
+      ALWAYS ON
       EXCLUDE_FROM_MAIN ON
       USES_TERMINAL 1
     )

diff  --git a/clang/cmake/caches/Release.cmake b/clang/cmake/caches/Release.cmake
index 1dfb1bc535bf1..9e6feb479d45f 100644
--- a/clang/cmake/caches/Release.cmake
+++ b/clang/cmake/caches/Release.cmake
@@ -30,7 +30,7 @@ endfunction()
 #
 # cmake -D LLVM_RELEASE_ENABLE_PGO=ON -C Release.cmake
 set(LLVM_RELEASE_ENABLE_LTO THIN CACHE STRING "")
-set(LLVM_RELEASE_ENABLE_PGO OFF CACHE BOOL "")
+set(LLVM_RELEASE_ENABLE_PGO ON CACHE BOOL "")
 set(LLVM_RELEASE_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 set(LLVM_RELEASE_ENABLE_PROJECTS "clang;lld;lldb;clang-tools-extra;bolt;polly;mlir;flang" CACHE STRING "")
 # Note we don't need to add install here, since it is one of the pre-defined

diff  --git a/clang/utils/perf-training/CMakeLists.txt b/clang/utils/perf-training/CMakeLists.txt
index 61df987da7139..49673790ff6e8 100644
--- a/clang/utils/perf-training/CMakeLists.txt
+++ b/clang/utils/perf-training/CMakeLists.txt
@@ -15,7 +15,7 @@ if(LLVM_BUILD_INSTRUMENTED)
   add_lit_testsuite(generate-profraw "Generating clang PGO data"
     ${CMAKE_CURRENT_BINARY_DIR}/pgo-data/
     EXCLUDE_FROM_CHECK_ALL
-    DEPENDS clang clear-profraw ${CLANG_PGO_TRAINING_DEPS}
+    DEPENDS clear-profraw
     )
 
   add_custom_target(clear-profraw
@@ -29,10 +29,21 @@ if(LLVM_BUILD_INSTRUMENTED)
   if(NOT LLVM_PROFDATA)
     message(STATUS "To enable merging PGO data LLVM_PROFDATA has to point to llvm-profdata")
   else()
-    add_custom_target(generate-profdata
+    add_custom_command(
+      OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/clang.profdata
+      # generate-profraw is a custom_target which are always considered stale.
+      # If we add it here to 'DEPENDS', then it will always execute and running
+      # ninja install && ninja check-all will result in the profile data being
+      # generated twice, and cause the ninja check-all build to fail with errors like:
+      # `ld.lld: error: Function Import: link error: linking module flags 'ProfileSummary': IDs have conflicting values in`
+      # Therefor we call the generate-profraw target manually as part of this custom
+      # command, which will only run if clang or ${CLANG_PGO_TRAINING_DEPS} are updated.
+      COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target generate-profraw
       COMMAND "${Python3_EXECUTABLE}" ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py merge ${LLVM_PROFDATA} ${CMAKE_CURRENT_BINARY_DIR}/clang.profdata ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_BINARY_DIR}/profiles/
       COMMENT "Merging profdata"
-      DEPENDS generate-profraw)
+      DEPENDS clang ${CLANG_PGO_TRAINING_DEPS}
+    )
+    add_custom_target(generate-profdata DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/clang.profdata)
     if (CLANG_PGO_TRAINING_DATA_SOURCE_DIR)
       llvm_ExternalProject_Add(generate-profraw-external ${CLANG_PGO_TRAINING_DATA_SOURCE_DIR}
               USE_TOOLCHAIN EXLUDE_FROM_ALL NO_INSTALL DEPENDS generate-profraw)