[PGO][HIP] Fix profile-only Windows link by gating ROCm interceptor macro - #200859
Conversation
…acro PR llvm#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#200111 (comment)
|
@llvm/pr-subscribers-pgo Author: Yaxun (Sam) Liu (yxsamliu) ChangesPR #200111 stops compiling InstrProfilingPlatformROCm.cpp (which defines the On ELF the declaration is weak, so the undefined symbol folds to null and the error LNK2019: unresolved external symbol Add the define only when PROFILE_HAS_HIP_INTERCEPTOR is true, i.e. the same Reported by zmodem: Full diff: https://github.com/llvm/llvm-project/pull/200859.diff 1 Files Affected:
diff --git a/compiler-rt/lib/profile/CMakeLists.txt b/compiler-rt/lib/profile/CMakeLists.txt
index de26177a4a2b7..77db2477bb7c6 100644
--- a/compiler-rt/lib/profile/CMakeLists.txt
+++ b/compiler-rt/lib/profile/CMakeLists.txt
@@ -158,12 +158,6 @@ if(COMPILER_RT_PROFILE_BAREMETAL)
-DCOMPILER_RT_PROFILE_BAREMETAL=1)
endif()
-if(COMPILER_RT_BUILD_PROFILE_ROCM)
- set(EXTRA_FLAGS
- ${EXTRA_FLAGS}
- -DCOMPILER_RT_BUILD_PROFILE_ROCM=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
@@ -190,6 +184,17 @@ if(NOT PROFILE_HAS_HIP_INTERCEPTOR)
list(REMOVE_ITEM PROFILE_SOURCES InstrProfilingPlatformROCm.cpp)
endif()
+# Only advertise the ROCm interceptor to InstrProfilingFile.c when its
+# definition (InstrProfilingPlatformROCm.cpp) is actually compiled into the
+# archive. Otherwise InstrProfilingFile.c references
+# __llvm_profile_hip_collect_device_data with no definition; on COFF/Windows
+# there is no weak-undefined fallback, so the link fails (see PR #200111).
+if(COMPILER_RT_BUILD_PROFILE_ROCM AND PROFILE_HAS_HIP_INTERCEPTOR)
+ set(EXTRA_FLAGS
+ ${EXTRA_FLAGS}
+ -DCOMPILER_RT_BUILD_PROFILE_ROCM=1)
+endif()
+
if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "amdgcn|nvptx")
append_list_if(COMPILER_RT_HAS_FFREESTANDING_FLAG -ffreestanding EXTRA_FLAGS)
append_list_if(COMPILER_RT_HAS_NOGPULIB_FLAG -nogpulib EXTRA_FLAGS)
|
jhuber6
left a comment
There was a problem hiding this comment.
This is getting a little convoluted
|
Thanks! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/42/builds/8986 Here is the relevant piece of the build log for the reference |
…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 #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)