ROCm / HIP

HIP: C++ Heterogeneous-Compute Interface for Portability
https://rocmdocs.amd.com/projects/HIP/
MIT License
3.73k stars 529 forks source link

HIP + ELFIO (in rocclr, in HIP and upstream) = a bit of a mess. #2259

Closed Artem-B closed 7 months ago

Artem-B commented 3 years ago

AMD has recentrly introduced a lot of new ELF flags.

These flags made it into:

Eventually HIP's own code started using them: https://github.com/ROCm-Developer-Tools/HIP/commit/c4beefe00b1dfa0100f02a9e99d4ea4fb52873da#diff-a5d22ac1ef5d8869cdff986cfeb4d69f3bfe99ce4ff57469df6f8684de3137fa

The problem is that HIP's own copy of elfio does not have these constants: https://github.com/ROCm-Developer-Tools/HIP/blob/rocm-4.1.x/include/hip/hcc_detail/elfio/elf_types.hpp I guess the only reason HIP builds now is that it picks up ROCclr's copy of ELFIO headers. If that's intentional, then it's not clear why the old copy of elfio still hangs around in HIP.

Nor did these constants make it into upstream ELFIO: https://github.com/serge1/ELFIO/blob/master/elfio/elf_types.hpp

It would be great to have these new ELF constants upstreamed to https://github.com/serge1/ELFIO/ and just use it as an external dependency. Compiling code using multiple different versions of the same library is a pretty good way to buy fair amount of trouble (ODR violations are never fun to troubleshoot). Using a private copy of a public library is double so as that would cause trouble for the end users who may need to use the upstream version of the ELFIO library.

ppanchad-amd commented 7 months ago

@Artem-B , Sorry for the lack of response. Please try latest ROCm 6.0.2 (HIP 6.0.32831) to see if your issue still exists? If resolved, please close the ticket. Thanks.

Artem-B commented 7 months ago

ROCclr is gone, so I guess the issue is obsolete now.

b-sumner commented 7 months ago

ROCclr is now part of this repo: https://github.com/ROCm/clr