search

ROCm / rocprofiler

ROC profiler library. Profiling with perf-counters and derived metrics.
https://rocm.docs.amd.com/projects/rocprofiler/en/latest/
MIT License
132 stars 49 forks source link

why are you linking to numa? #63

Closed haampie closed 2 months ago

haampie commented 2 years ago
  1. The tests are always linking to numa, without even running find_library (on any tagged version, not on the amd-master branch, confusingly)
  2. You can't turn off building the tests.

Can you please fix this?

libnuma is not a direct dependency of rocprofiler as far as I can see, it's a transient dependency: librocprofiler.so depends on libhsa-runtime64.so depends on libhsakmt.so depends on libnuma.so

Also hsa runtime has a cmake config file, so why don't you use it? My guess its someone added linking to libnuma because they had a static build of rocm libs? If you would use find_package(...) the flags will likely be correct including -lnuma if some libraries are static, and without -lnuma if shared.

haampie commented 2 years ago

https://github.com/ROCm-Developer-Tools/rocprofiler/commit/97c9efce38db1804235c9a95483a5eea189af6de yeah, this seems like a mistake, since linking to numa is not required for shared builds. Please use the HSA cmake config file and don't link to numa unconditionally. Pinging the commit author @kentrussell

kentrussell commented 2 years ago

It should be resolved in the next release. It was done as a workaround measure due to the thunk becoming static, and the CMake file being a little messy (and me not having the time to fix it properly). 5.0 should have it covered though. I'll leave this open in case the rocprofiler owners don't get the proper fix in for that release

haampie commented 2 years ago

great, thanks!

harkgill-amd commented 3 months ago

Hi @haampie, test/CMakeLists.txt no longer references numa as of ROCm 6.2. Could you please confirm if we can close out this ticket?