Re: Troubles with rocm-compilersupport/5.6+git20231212.4510c28-1~exp1
Hi Cory,
On 2024-01-15 23:16, Cordell Bloor wrote:
> I've pushed some updates to rocm-compilersupport on salsa. However,
> there are three issues that are blocking me from submitting an RFS.
>
> 1. Code Object V3 has been deprecated in LLVM, so sample COV3 binaries
> were checked in to the comgr repo to ensure the loader could still be
> tested once support is removed [1]. These binaries were generated from
> the sources in the repository. I don't think this is a DFSG issue
> because the binaries could be built from the sources in the tarball.
> Nevertheless, I'm thinking that perhaps I should repack the tarball to
> remove the binaries and patch comgr to continue to build the test data
> from source.
It is, at the very least, best practice to always rebuild the binaries.
In the long past, I think ftp-master did accept packages with some
pre-built binaries (like firmware), but I would always rebuild if only
to ensure that this is even possible.
> 2. The amd_comgr_get_isa_count function was exported with the wrong
> version in libamd_comgr2. The ABI of amd_comgr_get_isa_count was changed
> in 2.0, so it was supposed to added to the amd_comgr@2_0 symbol list and
> removed from the amd_comgr@1_8 symbol list. However, it was not removed,
> so it was (incorrectly) marked as amd_comgr@1_8 in comgr 2.0. Later, the
> duplicate was removed [2] and the symbol was therefore changed from
> amd_comgr@1_8 to amd_comgr@2_0. I've reverted the removal, but that will
> leave Debian's ABI inconsistent with upstream.
This is a tricky one, but I *think* it should be OK to just bump the symbol.
This would break current users of amd_comgr_get_isa_count@amd_comgr_1.8
but since the ABI has changed, current users would be broken anyway, right?
The super-safe thing to do would be to
(1) mark the current version as 2.0
(2) re-add the old function under some other name via a patch
(3) export (3) as amd_comgr_get_isa_count@amd_comgr_1.8
This process is described in [4], for example:
> The second GNU extension is to allow multiple versions of the same function to appear in a given shared library. In this way you can make an incompatible change to an interface without increasing the major version number of the shared library, while still allowing applications linked against the old interface to continue to function.
> The real mistake was back when comgr was bumped to 2.0, and arguably the
> removal of the wrong symbol is merely a bugfix that brings the actual
> ABI in line with the original intention. However, this is still quite
> unfortunate. I suppose maybe we could add both symbols? I'm not familiar
> with the exportmap that is used for the symbol versioning here, but
> perhaps there's a good way add an alias that handles this.
>
> 3. There's a few lines of code in the build file that have been included
> from a project with no license. The root build file is a rather annoying
> place to have such a problem, but I will probably repack the tarball to
> omit that code. The questionable code isn't useful for Debian in
> building the package anyway.
That sounds correct.
Best,
Christian
> [1]: https://github.com/ROCm/ROCm-CompilerSupport/commit/3c51531db0939006f589cecf01f748361f151692
> [2]: https://github.com/ROCm/ROCm-CompilerSupport/commit/af5b350b41bd8e7902d57b74531d2dcab6c1f921
> [3]: https://salsa.debian.org/rocm-team/rocm-compilersupport/-/blob/upstream/5.6+git20231212.4510c28/lib/comgr/CMakeLists.txt#L6-26
[4]
Reply to: