Skip to content

Commit

Permalink
[clang][cmake] Fixes for PGO builds when invoking ninja twice (#92591)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tstellar authored Jun 10, 2024
1 parent 52050f3 commit be06761
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 16 deletions.
18 changes: 6 additions & 12 deletions clang/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
2 changes: 1 addition & 1 deletion clang/cmake/caches/Release.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 14 additions & 3 deletions clang/utils/perf-training/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down

0 comments on commit be06761

Please sign in to comment.