[PGO][HIP] Skip ROCm interceptor in profile-only compiler-rt builds - #200111
Conversation
|
@llvm/pr-subscribers-pgo Author: Yaxun (Sam) Liu (yxsamliu) ChangesPR #177665 made Fix: gate the object-lib merge on the Verified:
Full diff: https://github.com/llvm/llvm-project/pull/200111.diff 1 Files Affected:
diff --git a/compiler-rt/lib/profile/CMakeLists.txt b/compiler-rt/lib/profile/CMakeLists.txt
index 36e6b5a6a21c1..2b09548bb3a1e 100644
--- a/compiler-rt/lib/profile/CMakeLists.txt
+++ b/compiler-rt/lib/profile/CMakeLists.txt
@@ -156,8 +156,18 @@ if(COMPILER_RT_PROFILE_BAREMETAL)
-DCOMPILER_RT_PROFILE_BAREMETAL=1)
endif()
+# The HIP host interceptor in InstrProfilingPlatformROCm.cpp pulls in
+# RTInterception + sanitizer_common object libs. Those targets are only created
+# when COMPILER_RT_BUILD_SANITIZERS / _MEMPROF / _XRAY / _CTX_PROFILE is enabled
+# (see lib/CMakeLists.txt). In a profile-only build the targets do not exist;
+# skip both the object-lib merge and the ROCm source file so the static archive
+# remains self-contained.
set(PROFILE_OBJECT_LIBS)
-if(COMPILER_RT_HAS_INTERCEPTION AND NOT COMPILER_RT_PROFILE_BAREMETAL)
+set(PROFILE_HAS_HIP_INTERCEPTOR FALSE)
+if(COMPILER_RT_HAS_INTERCEPTION AND NOT COMPILER_RT_PROFILE_BAREMETAL
+ AND TARGET RTInterception.${COMPILER_RT_DEFAULT_TARGET_ARCH}
+ AND TARGET RTSanitizerCommon.${COMPILER_RT_DEFAULT_TARGET_ARCH}
+ AND TARGET RTSanitizerCommonLibc.${COMPILER_RT_DEFAULT_TARGET_ARCH})
# RTInterception references __sanitizer_internal_{memcpy,memset,memmove} and other
# sanitizer_common symbols; merge the same object libs as clang_rt.cfi (without
# coverage/symbolizer) so -fprofile-instr-generate links stay self-contained.
@@ -165,6 +175,11 @@ if(COMPILER_RT_HAS_INTERCEPTION AND NOT COMPILER_RT_PROFILE_BAREMETAL)
RTInterception
RTSanitizerCommon
RTSanitizerCommonLibc)
+ set(PROFILE_HAS_HIP_INTERCEPTOR TRUE)
+endif()
+
+if(NOT PROFILE_HAS_HIP_INTERCEPTOR)
+ list(REMOVE_ITEM PROFILE_SOURCES InstrProfilingPlatformROCm.cpp)
endif()
if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "amdgcn|nvptx")
@@ -185,7 +200,7 @@ if(MSVC)
# built with MultiThreadedDLL (/MD) — see interception/CMakeLists.txt and
# sanitizer_common/CMakeLists.txt. Mixing /MD objects into a /MT libclang_rt.profile
# yields LNK2019 (__imp__stricmp from interception_win.cpp) and LNK4098 in Profile-*.
- if(COMPILER_RT_HAS_INTERCEPTION AND NOT COMPILER_RT_PROFILE_BAREMETAL)
+ if(PROFILE_HAS_HIP_INTERCEPTOR)
set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDLL)
else()
set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded)
|
PR llvm#177665 made libclang_rt.profile.a merge RTInterception + sanitizer_common object libs to support InstrProfilingPlatformROCm.cpp's hipModuleLoad interceptor. Those object-lib targets are only created when COMPILER_RT_BUILD_SANITIZERS / _MEMPROF / _XRAY / _CTX_PROFILE is enabled (see lib/CMakeLists.txt), so a profile-only configuration fails at configure time: Error evaluating generator expression: $<TARGET_OBJECTS:RTInterception.x86_64> Objects of target "RTInterception.x86_64" referenced but no such target exists. Gate the object-lib merge on the targets actually existing, and drop InstrProfilingPlatformROCm.cpp from PROFILE_SOURCES in that case so the static archive stays self-contained. The host-side hook __llvm_profile_hip_collect_device_data is already declared weak in InstrProfilingFile.c, so its absence is fine at link time.
feb727e to
fc23037
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/37444 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/20868 Here is the relevant piece of the build log for the reference |
|
This broke our builds on Windows. I guess it relates to this:
because we get undefined symbol errors for Here's a reproducer: |
Thanks. got a fix #200859 |
…acro (#200859) PR #200111 stops compiling InstrProfilingPlatformROCm.cpp (which defines the HIP GPU helper __llvm_profile_hip_collect_device_data) in profile-only builds. But the compile define -DCOMPILER_RT_BUILD_PROFILE_ROCM=1 was still added whenever the COMPILER_RT_BUILD_PROFILE_ROCM option was on (the default), so InstrProfilingFile.c still referenced the helper from __llvm_profile_write_file even though it was never built. On ELF the declaration is weak, so the undefined symbol folds to null and the address-guarded call is skipped. COFF/Windows has no such fallback: error LNK2019: unresolved external symbol __llvm_profile_hip_collect_device_data referenced in function __llvm_profile_write_file Add the define only when PROFILE_HAS_HIP_INTERCEPTOR is true, i.e. the same condition that keeps InstrProfilingPlatformROCm.cpp in the archive, so the macro is defined iff the helper is actually compiled in. Reported by zmodem: #200111 (comment)
…terceptor macro (#200859) PR #200111 stops compiling InstrProfilingPlatformROCm.cpp (which defines the HIP GPU helper __llvm_profile_hip_collect_device_data) in profile-only builds. But the compile define -DCOMPILER_RT_BUILD_PROFILE_ROCM=1 was still added whenever the COMPILER_RT_BUILD_PROFILE_ROCM option was on (the default), so InstrProfilingFile.c still referenced the helper from __llvm_profile_write_file even though it was never built. On ELF the declaration is weak, so the undefined symbol folds to null and the address-guarded call is skipped. COFF/Windows has no such fallback: error LNK2019: unresolved external symbol __llvm_profile_hip_collect_device_data referenced in function __llvm_profile_write_file Add the define only when PROFILE_HAS_HIP_INTERCEPTOR is true, i.e. the same condition that keeps InstrProfilingPlatformROCm.cpp in the archive, so the macro is defined iff the helper is actually compiled in. Reported by zmodem: llvm/llvm-project#200111 (comment)
…terceptor macro (#200859) PR #200111 stops compiling InstrProfilingPlatformROCm.cpp (which defines the HIP GPU helper __llvm_profile_hip_collect_device_data) in profile-only builds. But the compile define -DCOMPILER_RT_BUILD_PROFILE_ROCM=1 was still added whenever the COMPILER_RT_BUILD_PROFILE_ROCM option was on (the default), so InstrProfilingFile.c still referenced the helper from __llvm_profile_write_file even though it was never built. On ELF the declaration is weak, so the undefined symbol folds to null and the address-guarded call is skipped. COFF/Windows has no such fallback: error LNK2019: unresolved external symbol __llvm_profile_hip_collect_device_data referenced in function __llvm_profile_write_file Add the define only when PROFILE_HAS_HIP_INTERCEPTOR is true, i.e. the same condition that keeps InstrProfilingPlatformROCm.cpp in the archive, so the macro is defined iff the helper is actually compiled in. Reported by zmodem: llvm/llvm-project#200111 (comment)
…201416) This broke profiling builds on Windows by switching the profile library to link against the dynamic CRT; see discussion on the PR. There were already a number of issues reported and fixed after this PR. Rather than piling on the fixes (and this one may need some work), revert back to green for now to let the project recover. This reverts commit 5db1364. Additionally, this reverts the followup PRs in 635e120, 2766733, 4c33844, and 5eca8b6: "[PGO][HIP] Stop pulling ROCm.o into every PGO host link (#200101)" "[compiler-rt][profile] Add COMPILER_RT_BUILD_PROFILE_ROCM option (#200127)" "[PGO][HIP] Skip ROCm interceptor in profile-only compiler-rt builds (#200111)" "[PGO][HIP] Fix profile-only Windows link by gating ROCm interceptor macro (#200859)"
PR #177665 made
libclang_rt.profile.amergeRTInterceptionandsanitizer_commonobject libs to support thehipModuleLoad*host interceptor added inInstrProfilingPlatformROCm.cpp. Those object-lib targets are only created whenCOMPILER_RT_BUILD_SANITIZERS / _MEMPROF / _XRAY / _CTX_PROFILEis enabled (seecompiler-rt/lib/CMakeLists.txt), so a profile-only configuration fails at configure time:Fix: gate the object-lib merge on the
RTInterception.<arch>/RTSanitizerCommon*.<arch>targets actually existing, and dropInstrProfilingPlatformROCm.cppfromPROFILE_SOURCESin that case so the static archive stays self-contained. The host-side hook__llvm_profile_hip_collect_device_datais already declared weak inInstrProfilingFile.c(PR #200101), so its absence is fine at link time.Verified:
-DCOMPILER_RT_BUILD_SANITIZERS=OFF -DCOMPILER_RT_BUILD_MEMPROF=OFF -DCOMPILER_RT_BUILD_XRAY=OFF -DCOMPILER_RT_BUILD_CTX_PROFILE=OFF -DCOMPILER_RT_BUILD_PROFILE=ON) now configures and links cleanly.check-profile134/134 pass.