Skip to content
This repository has been archived by the owner on Jan 26, 2024. It is now read-only.

Do librocclr really need libamd_comgr? #38

Open
littlewu2508 opened this issue Sep 1, 2022 · 1 comment
Open

Do librocclr really need libamd_comgr? #38

littlewu2508 opened this issue Sep 1, 2022 · 1 comment

Comments

@littlewu2508
Copy link

Related to ROCm/hipamd#44

I'm maintaining hip and rocm-opencl-runtime package in Gentoo, and recently, I find although in wirtten cmake, rocclr is linked to libamd_comgr, in reality linker dropped it since there is not function from libamd_comgr called.

I browsed ROCclr source codes by ag amd::Comgr:: | grep -o -E "amd::Comgr::[^\(]*\(" | sort -u, finding these functions are defined in amd_comgr.h, not comgr .cpp files, so librocclr does not have to link libamd_comgr. Is this just a coincidence, or a design? For the later one, I can make hip and rocm-opencl-runtime independent of rocm-comgr, since some tests of rocm-comgr are only available when hip is installed.

@Mystro256
Copy link
Contributor

ROCclr loads libamd_comgr.so.2, see device/comgrctx.cpp. There's some compatibility code to allow this to also work on Windows, but there's no dynamic linking if you trace the code back. I believe it's done this way to make it more portable, but I'm not 100% sure as I'm not a developer.

If you package rocm-opencl or hipamd, which statically compile in ROCclr, you need to make sure you add an explicit requirement on comgr. I'm not sure about Gentoo, but on Fedora and Debian, runtime requirements are usually determined automatically via dynamic linking requirements. Since ROCclr does not dynamically link against comgr, a requirement must be specified explicitly.

E.g. from Fedora:
https://src.fedoraproject.org/rpms/rocm-opencl/blob/rawhide/f/rocm-opencl.spec#_37

ROCclr is not a stand alone library, and is designed to be a static lib within rocm's opencl/hip to act as an extraction layer of sorts (works against both Windows and Linux backends). As well, comgr is a compiler interface for both libraries, so I don't think it can be dropped.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants