Skip to content

fix(compiler): pin host LLVM_ENABLE_PER_TARGET_RUNTIME_DIR to OFF - #7559

Merged
lamb-j merged 2 commits into
mainfrom
users/lambj/llvm-host-per-target-runtime-dir
Aug 27, 2026
Merged

lamb-j merged 2 commits into
mainfrom
users/lambj/llvm-host-per-target-runtime-dir

Conversation

@lamb-j

@lamb-j lamb-j commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Passes -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF for the host triple in the amd-llvm subproject's CMAKE_ARGS.

The subproject declares its interface directories as a flat host runtime tree — INSTALL_DESTINATION "lib/llvm", INTERFACE_LINK_DIRS "lib/llvm/lib", INTERFACE_INSTALL_RPATH_DIRS "lib/llvm/lib" — but never sets the knob that controls whether LLVM actually installs there. Upstream defaults it ON for BSD|Linux|OS390|AIX, which puts libomp.so, libomptarget, libunwind and libc++abi in lib/llvm/lib/<host-triple>/, one directory below the RUNPATH TheRock hands its own consumers.

From a shipped 7.12 tree:

$ readelf -d bin/rocsparse-test | grep -E 'NEEDED.*omp|RUNPATH'
 (NEEDED)   Shared library: [libomp.so]
 (RUNPATH)  [$ORIGIN/../lib/rocm_sysdeps/lib:$ORIGIN/../lib/llvm/lib:$ORIGIN/../lib]

$ ls lib/llvm/lib/libomp.so
lib/llvm/lib/libomp.so

With the default in effect, rocsparse-test and rocFFT fail at load with libomp.so: cannot open shared object file.

This has not bitten us because amd-staging pins the default OFF unconditionally in llvm/CMakeLists.txt, right after the platform block. That makes TheRock's install layout depend on a compiler-side patch it does not own — any LLVM branch without the override, including stock upstream, produces a broken dist. The knob is a CACHE BOOL, so setting it on the configure line needs no LLVM source change and makes the requirement explicit at the point that relies on it.

Setting it unconditionally is a no-op on Windows, where the upstream default is already OFF.

This is the host triple only. RUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON — the device runtimes, which TheRock does want per-target — is unchanged; the two now sit adjacent with a comment distinguishing them.

ISSUE ID: #7557

The amd-llvm subproject declares INTERFACE_LINK_DIRS and
INTERFACE_INSTALL_RPATH_DIRS as lib/llvm/lib, so every consumer links and
runs against a flat host runtime directory. LLVM defaults
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR to ON on Linux, which installs libomp.so,
libomptarget, libunwind and libc++abi into lib/llvm/lib/<host-triple>/
instead - outside the RUNPATH TheRock hands out. rocsparse-test, for example,
has NEEDED libomp.so and RUNPATH $ORIGIN/../lib/llvm/lib, and fails at load
with "libomp.so: cannot open shared object file".

Builds have not hit this because amd-staging carries an unconditional
override of the default in llvm/CMakeLists.txt. TheRock should not depend on
a compiler-side patch for its own install layout: the knob is a CACHE BOOL,
so pinning it on the configure line makes the requirement explicit and keeps
the build correct against a stock upstream LLVM.

This is the host triple only. The device runtimes keep the per-target layout
via RUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_PER_TARGET_RUNTIME_DIR, unchanged.

ISSUE ID: #7557
@therock-pr-bot

therock-pr-bot Bot commented Aug 21, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

Comment thread compiler/CMakeLists.txt Resolved

@kasaurov kasaurov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, very reasonable solution.

@lamb-j
lamb-j merged commit c3621af into main Aug 27, 2026
172 of 176 checks passed
@lamb-j
lamb-j deleted the users/lambj/llvm-host-per-target-runtime-dir branch August 27, 2026 19:16
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Aug 27, 2026
ronlieb pushed a commit to ROCm/llvm-project that referenced this pull request Aug 30, 2026
…rride (#4179)

## Downstream delta removed

`amd-staging` carries a one-line AMD-local override that clobbers
upstream's platform default immediately after it is computed:

```cmake
if(CMAKE_SYSTEM_NAME MATCHES "BSD|Linux|OS390|AIX")
  set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON)
else()
  set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default OFF)
endif()
set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default OFF)   # <-- removed
```

After this change `llvm/CMakeLists.txt` has **zero**
`LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` divergence from `upstream/main`.
Upstream is moving toward keeping this ON by default and dropping the
option entirely, so carrying the override moves us further from upstream
over time.

## Why it existed

ROCm consumers assumed the flat `lib/llvm/lib` layout. With the
per-target directory on, `libomp.so` moves to
`lib/llvm/lib/<host-triple>/` while consumer RUNPATHs still point at
`lib/llvm/lib` — the library ships but cannot be loaded.

## Why TheRock is unaffected

TheRock passes `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR` explicitly
(ROCm/TheRock#7559; ROCm/TheRock#7082 moves it to `ON` and fixes the
consumers). A command-line `-D` beats a `set(... CACHE ...)` without
`FORCE`, so TheRock's value wins regardless of the default here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants