https://github.com/tstellar created
https://github.com/llvm/llvm-project/pull/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.
>From 33d373044e247a568d2fce7371a6ac1c0527f1a2 Mon Sep 17 00:00:00 2001
From: Tom Stellard <tstel...@redhat.com>
Date: Thu, 16 May 2024 13:49:40 +0000
Subject: [PATCH] [clang][cmake] Fixes for PGO builds when invoking ninja twice
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.
---
clang/CMakeLists.txt | 18 ++++++------------
clang/cmake/caches/Release.cmake | 2 +-
clang/utils/perf-training/CMakeLists.txt | 17 ++++++++++++++---
3 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index c20ce47a12abb..b4cf83d18c4e6 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -849,23 +849,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 c0bfcbdfc1c2a..d40a84cf2a5f0 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)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
