Skip to content

[PGO][HIP] Stop pulling ROCm.o into every PGO host link - #200101

Merged
yxsamliu merged 2 commits into
llvm:mainfrom
yxsamliu:amd/dev/yaxunl/PR-177665-host-side-interceptor-fix
May 28, 2026
Merged

[PGO][HIP] Stop pulling ROCm.o into every PGO host link#200101
yxsamliu merged 2 commits into
llvm:mainfrom
yxsamliu:amd/dev/yaxunl/PR-177665-host-side-interceptor-fix

Conversation

@yxsamliu

Copy link
Copy Markdown
Contributor

PR #177665 added an unconditional extern reference to __llvm_profile_hip_collect_device_data from InstrProfilingFile.c, which forces InstrProfilingPlatformROCm.o (and its sanitizer_common / interception dependencies) out of libclang_rt.profile.a in every PGO binary. That breaks bots without -lpthread and races dlsym/PLT state in non-HIP programs via the interceptor constructor.

Fix:

  • Declare the hook COMPILER_RT_WEAK and gate the call on its address. No COMPILER_RT_VISIBILITY: a hidden weak-undef function would be non-preemptible and the address test would fold to true.
  • Gate installHipModuleInterceptors on dlsym(hipModuleLoad) so the constructor is a no-op if ROCm.o is still pulled in.

Fixes:

Verified:

  • check-profile 134/134 pass.
  • nm on a non-HIP clang -fprofile-generate binary: zero installHip/ROCm/sanitizer/hip_collect symbols.
  • HIP offload PGO end-to-end on gfx1101 (compile → run → llvm-profdata mergellvm-cov) still works; interceptor installs, device profile collected via shared API.

PR llvm#177665 added an unconditional extern reference to
__llvm_profile_hip_collect_device_data from InstrProfilingFile.c, which
forces InstrProfilingPlatformROCm.o (and its sanitizer_common /
interception dependencies) out of libclang_rt.profile.a in every PGO
binary. That breaks bots without -lpthread and races dlsym/PLT state in
non-HIP programs via the interceptor constructor.

Fix:
- Declare the hook COMPILER_RT_WEAK and gate the call on its address.
  No COMPILER_RT_VISIBILITY: a hidden weak-undef function would be
  non-preemptible and the address test would fold to true.
- Gate installHipModuleInterceptors on dlsym(hipModuleLoad) so the
  constructor is a no-op if ROCm.o is still pulled in.

Fixes:
  https://lab.llvm.org/buildbot/#/builders/66/builds/31311
  https://lab.llvm.org/buildbot/#/builders/174/builds/36180
@llvmorg-github-actions llvmorg-github-actions Bot added compiler-rt PGO Profile Guided Optimizations labels May 28, 2026
@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-pgo

Author: Yaxun (Sam) Liu (yxsamliu)

Changes

PR #177665 added an unconditional extern reference to __llvm_profile_hip_collect_device_data from InstrProfilingFile.c, which forces InstrProfilingPlatformROCm.o (and its sanitizer_common / interception dependencies) out of libclang_rt.profile.a in every PGO binary. That breaks bots without -lpthread and races dlsym/PLT state in non-HIP programs via the interceptor constructor.

Fix:

  • Declare the hook COMPILER_RT_WEAK and gate the call on its address. No COMPILER_RT_VISIBILITY: a hidden weak-undef function would be non-preemptible and the address test would fold to true.
  • Gate installHipModuleInterceptors on dlsym(hipModuleLoad) so the constructor is a no-op if ROCm.o is still pulled in.

Fixes:

Verified:

  • check-profile 134/134 pass.
  • nm on a non-HIP clang -fprofile-generate binary: zero installHip/ROCm/sanitizer/hip_collect symbols.
  • HIP offload PGO end-to-end on gfx1101 (compile → run → llvm-profdata mergellvm-cov) still works; interceptor installs, device profile collected via shared API.

Full diff: https://github.com/llvm/llvm-project/pull/200101.diff

2 Files Affected:

  • (modified) compiler-rt/lib/profile/InstrProfilingFile.c (+13-7)
  • (modified) compiler-rt/lib/profile/InstrProfilingPlatformROCm.cpp (+7)
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index 78e1edb54768f..8d068c103574f 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -41,9 +41,14 @@
 #include "InstrProfilingPort.h"
 #include "InstrProfilingUtil.h"
 
-/* HIP / offload collection hook implemented in InstrProfilingPlatformROCm.c.
- * It is a no-op when no offload profile data was registered. */
-extern int __llvm_profile_hip_collect_device_data(void);
+/* Weak so non-HIP programs do not force InstrProfilingPlatformROCm.o (and its
+ * transitive sanitizer_common / interception dependencies) into the host link
+ * out of libclang_rt.profile.a. HIP programs emit strong references to other
+ * ROCm-runtime symbols (e.g. __llvm_profile_offload_register_shadow_variable)
+ * that pull in the strong definition.
+ * No COMPILER_RT_VISIBILITY: a hidden weak-undefined symbol is non-preemptible
+ * and the address test at the call site would fold to true. */
+COMPILER_RT_WEAK int __llvm_profile_hip_collect_device_data(void);
 
 /* From where is profile name specified.
  * The order the enumerators define their
@@ -1202,10 +1207,11 @@ int __llvm_profile_write_file(void) {
   if (rc)
     PROF_ERR("Failed to write file \"%s\": %s\n", Filename, strerror(errno));
 
-  /* No-op when no HIP shadow variables or dynamic modules are registered,
-   * or when the HIP runtime is not loaded. Warning on failure is handled
-   * inside the callee so non-HIP programs do not see spurious noise. */
-  (void)__llvm_profile_hip_collect_device_data();
+  /* Weak: only invoked when InstrProfilingPlatformROCm.o is in the link,
+   * which happens when the program references other ROCm-runtime symbols
+   * (HIP-with-PGO). Warning on failure is handled inside the callee. */
+  if (__llvm_profile_hip_collect_device_data)
+    (void)__llvm_profile_hip_collect_device_data();
 
   // Restore SIGKILL.
   if (PDeathSig == 1)
diff --git a/compiler-rt/lib/profile/InstrProfilingPlatformROCm.cpp b/compiler-rt/lib/profile/InstrProfilingPlatformROCm.cpp
index 9e234da5b5cc4..ee00c572e3a42 100644
--- a/compiler-rt/lib/profile/InstrProfilingPlatformROCm.cpp
+++ b/compiler-rt/lib/profile/InstrProfilingPlatformROCm.cpp
@@ -24,6 +24,7 @@ extern "C" {
 #define WIN32_LEAN_AND_MEAN
 #include <windows.h>
 #else
+#include <dlfcn.h>
 #include <pthread.h>
 #endif
 
@@ -878,6 +879,12 @@ INTERCEPTOR(int, hipModuleUnload, void *module) {
 }
 
 __attribute__((constructor)) static void installHipModuleInterceptors() {
+  /* Skip when the HIP runtime is not loaded. INTERCEPT_FUNCTION uses the
+   * sanitizer interception framework, which can perturb dlsym/PLT state for
+   * the rest of the process even when the target symbol is absent; non-HIP
+   * programs linked with libclang_rt.profile.a must see zero side effects. */
+  if (!dlsym(RTLD_DEFAULT, "hipModuleLoad"))
+    return;
   if (!INTERCEPT_FUNCTION(hipModuleLoad))
     return;
   if (isVerboseMode())

@yxsamliu
yxsamliu requested review from jhuber6 and ronlieb May 28, 2026 02:08
@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown

🪟 Windows x64 Test Results

  • 1631 tests passed
  • 1342 tests skipped

✅ The build succeeded and all tests passed.

@yxsamliu
yxsamliu merged commit 635e120 into llvm:main May 28, 2026
10 checks passed
yxsamliu added a commit that referenced this pull request May 29, 2026
…200111)

PR #177665 made `libclang_rt.profile.a` merge `RTInterception` and
`sanitizer_common` object libs to support the `hipModuleLoad*` host
interceptor added in `InstrProfilingPlatformROCm.cpp`. Those object-lib
targets are only created when `COMPILER_RT_BUILD_SANITIZERS / _MEMPROF /
_XRAY / _CTX_PROFILE` is enabled (see `compiler-rt/lib/CMakeLists.txt`),
so a profile-only configuration fails at configure time:

```
CMake Error at cmake/Modules/AddCompilerRT.cmake:365 (add_library):
  Error evaluating generator expression:
    $<TARGET_OBJECTS:RTInterception.x86_64>
  Objects of target "RTInterception.x86_64" referenced but no such target
  exists.
Call Stack (most recent call first):
  lib/profile/CMakeLists.txt:233 (add_compiler_rt_runtime)
```

Fix: gate the object-lib merge on the `RTInterception.<arch>` /
`RTSanitizerCommon*.<arch>` 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` (PR #200101), so its absence is fine at link
time.

Verified:
- Profile-only standalone compiler-rt build
(`-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.
- Default runtimes build still merges the object libs and produces the
interceptor; `check-profile` 134/134 pass.
zmodem added a commit that referenced this pull request Jun 3, 2026
…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)"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler-rt PGO Profile Guided Optimizations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants