From 77971d0debd082ec2b4bbdabcdc5f797cad96430 Mon Sep 17 00:00:00 2001 From: Willow Lopez <100782273+Oxygen56@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:19:29 +0800 Subject: [PATCH] fix(rocm): prefer system LLVM runtime on native Linux (#7448) * fix(rocm): prefer system LLVM runtime on native Linux * Fix/adjust the nested LLVM probe for PR #7448: lib64 hosts and non-directories Two gaps found while simulating the fix against real ROCm layouts. 1. lib64 hosts got no LLVM dir. The candidate was built from the HSA dir, so a host with libhsa-runtime64 under lib64 probed /lib64/llvm/lib. ROCm installs LLVM under /lib/llvm regardless, so that host kept binding system libamd_comgr to the bundle's libLLVM: exactly the bug #7446 reports. Probe both spellings, the HSA dir's own first so a genuine lib64 layout still wins. When lib_sub is already "lib" the seen set collapses them. 2. os.path.exists accepted a non-directory. The serve-time caller joins these straight into LD_LIBRARY_PATH with no is-dir filter, so a file named llvm/lib reached the loader. os.path.isdir instead. Verified on a 27-case matrix built from real directory trees (not mocks), run on both Windows and Linux against three revisions: main, this PR as-is, and this commit. Zero regressions and zero reorderings of the pre-existing entries in every case, and the installer and launcher copies never disagree. The lib64 case goes [lib64] -> [lib64, lib/llvm/lib]; the file case drops the bogus entry; a symlinked llvm/lib resolves correctly on Linux. End-to-end loader check: built real ELF objects mirroring the shipped bundle (RUNPATH=$ORIGIN, an incomplete libLLVM.so.23.0git next to llama-server, system comgr from /opt/rocm/lib) and reproduced the reported failure verbatim, then confirmed the prepend clears it: before undefined symbol: LLVMInitializeSPIRVTarget -> after exit 0 Test helper now patches os.path.isdir alongside os.path.exists, else every fake host reports its nested llvm dir as missing. New cases: lib64 finding llvm under lib, lib64 preferring its own when both exist, and a real-filesystem check that a non-directory is not prepended. Removing the lib fallback from one copy reddens three tests including the two-copy parity guard. tests/studio/install: 1361 passed on Linux, 4 pre-existing environmental failures unchanged (3 managed-node-runtime under root, 1 the real /opt/rocm case already covered by #7397). 30/30 on the helper suite on Windows and Linux. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Daniel Han Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- studio/backend/core/inference/llama_cpp.py | 9 +++ studio/install_llama_prebuilt.py | 9 +++ .../test_rocm_native_linux_lib_dirs.py | 79 ++++++++++++++++++- 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index 47e46405be..f76afce9f4 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -309,6 +309,15 @@ def _native_linux_system_rocm_lib_dirs(binary_dir: str = "") -> "list[str]": os.path.join(d, "libhsa-runtime64.so.1") ): out.append(d) + # ROCm keeps LLVM's versioned runtime under /lib/llvm, so a + # lib64 host still finds it under lib. Probe both and keep them + # ahead of the bundle, else system libamd_comgr binds to the + # bundle's incompatible libLLVM.so.*. + for _sub in (lib_sub, "lib"): + llvm_lib = os.path.join(base, _sub, "llvm", "lib") + if llvm_lib not in seen and os.path.isdir(llvm_lib): + seen.add(llvm_lib) + out.append(llvm_lib) return out diff --git a/studio/install_llama_prebuilt.py b/studio/install_llama_prebuilt.py index 529b90c3e3..dd64d74d7d 100644 --- a/studio/install_llama_prebuilt.py +++ b/studio/install_llama_prebuilt.py @@ -4807,6 +4807,15 @@ def _native_linux_system_rocm_lib_dirs(binary_dir: str = "") -> list[str]: os.path.join(d, "libhsa-runtime64.so.1") ): out.append(d) + # ROCm keeps LLVM's versioned runtime under /lib/llvm, so a + # lib64 host still finds it under lib. Probe both and keep them + # ahead of the bundle, else system libamd_comgr binds to the + # bundle's incompatible libLLVM.so.*. + for _sub in (lib_sub, "lib"): + llvm_lib = os.path.join(base, _sub, "llvm", "lib") + if llvm_lib not in seen and os.path.isdir(llvm_lib): + seen.add(llvm_lib) + out.append(llvm_lib) return out diff --git a/tests/studio/install/test_rocm_native_linux_lib_dirs.py b/tests/studio/install/test_rocm_native_linux_lib_dirs.py index 9b95af88b0..39663bbc19 100644 --- a/tests/studio/install/test_rocm_native_linux_lib_dirs.py +++ b/tests/studio/install/test_rocm_native_linux_lib_dirs.py @@ -124,7 +124,12 @@ def _call( tmp_path factory branches on it, so a session-wide patch breaks the fixture on a Windows test host.""" with patch.object(sys, "platform", platform): - with patch("os.path.exists", _fake_exists(present)): + # isdir too: the llvm probe requires a directory, so a fake host that only + # answers exists() would report every nested llvm dir as missing. + with ( + patch("os.path.exists", _fake_exists(present)), + patch("os.path.isdir", _fake_exists(present)), + ): return _norm(impl(str(bundle))) @@ -260,6 +265,78 @@ class TestNativeLinuxRootResolution: for where, impl in _impls().items(): assert self._run(impl, bundle_dir, present) == ["/opt/rocm/lib64"], where + def test_nested_llvm_runtime_follows_system_rocm_lib(self, bundle_dir): + """#7446: libamd_comgr depends on ROCm's versioned LLVM runtime, which is + installed below lib/llvm/lib rather than directly in lib.""" + present = { + "/dev/kfd", + "/opt/rocm/lib/libhsa-runtime64.so", + "/opt/rocm/lib/llvm/lib", + } + for where, impl in _impls().items(): + assert self._run(impl, bundle_dir, present) == [ + "/opt/rocm/lib", + "/opt/rocm/lib/llvm/lib", + ], where + + def test_lib64_host_still_finds_llvm_under_lib(self, bundle_dir): + """ROCm puts LLVM under /lib/llvm even where HSA lives in lib64, so + deriving the llvm dir from the HSA dir alone would miss it and leave + libamd_comgr binding to the bundle's libLLVM.""" + present = { + "/dev/kfd", + "/opt/rocm/lib64/libhsa-runtime64.so", + "/opt/rocm/lib/llvm/lib", + } + for where, impl in _impls().items(): + assert self._run(impl, bundle_dir, present) == [ + "/opt/rocm/lib64", + "/opt/rocm/lib/llvm/lib", + ], where + + def test_lib64_host_prefers_its_own_llvm_dir_when_both_exist(self, bundle_dir): + present = { + "/dev/kfd", + "/opt/rocm/lib64/libhsa-runtime64.so", + "/opt/rocm/lib64/llvm/lib", + "/opt/rocm/lib/llvm/lib", + } + for where, impl in _impls().items(): + assert self._run(impl, bundle_dir, present) == [ + "/opt/rocm/lib64", + "/opt/rocm/lib64/llvm/lib", + "/opt/rocm/lib/llvm/lib", + ], where + + def test_llvm_path_that_is_a_file_is_not_prepended(self, tmp_path, bundle_dir): + """Real filesystem: the serve-time caller joins these straight into + LD_LIBRARY_PATH without an is-dir filter, so a non-directory must not + reach it.""" + root = tmp_path / "rocm" + (root / "lib").mkdir(parents = True) + (root / "lib" / "libhsa-runtime64.so").write_text("") + (root / "lib" / "llvm").mkdir() + (root / "lib" / "llvm" / "lib").write_text("not a directory") + real_exists = os.path.exists + # Pin both device nodes: a WSL test host really has /dev/dxg, which would + # take the WSL early-return and make this pass for the wrong reason. + pinned = {"/dev/kfd": True, "/dev/dxg": False} + + def _exists(p): + return pinned.get(str(p), None) if str(p) in pinned else real_exists(p) + + # A test host may itself have a real /opt/rocm (the default candidate), so + # assert on the bogus entry rather than on the whole list. + for where, impl in _impls().items(): + with ( + patch.object(sys, "platform", "linux"), + patch.dict(os.environ, {"ROCM_PATH": str(root)}, clear = True), + patch("os.path.exists", _exists), + ): + out = impl(str(bundle_dir)) + assert str(root / "lib") in out, where + assert str(root / "lib" / "llvm" / "lib") not in out, where + def test_lib_precedes_lib64_when_both_exist(self, bundle_dir): present = { "/dev/kfd",