[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Troubles with rocm-compilersupport/5.6+git20231212.4510c28-1~exp1



Sorry, somehow managed to hit send + confirm too fast.

On 2024-01-21 10:59, Christian Kastner wrote:
> On 2024-01-15 23:16, Cordell Bloor wrote:
>
>> 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.

this missed:

> To do this, you must use multiple ‘.symver’ directives in the source file. Here is an example:
> 
> __asm__(".symver original_foo,foo@");
> __asm__(".symver old_foo,foo@VERS_1.1");
> __asm__(".symver old_foo1,foo@VERS_1.2");
> __asm__(".symver new_foo,foo@@VERS_2.0");


Thought the text suggests that this is a GNU extension.

I'd probably just change the symbol given the reasoning above (it's
broken either way), but I would wait for second opinions.

> 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] https://sourceware.org/binutils/docs/ld/VERSION.html


Reply to: