axom icon indicating copy to clipboard operation
axom copied to clipboard

Patching adiak::mpi with dl

Open chapman39 opened this issue 10 months ago • 5 comments

I noticed this issue while testing Axom v0.9.1 in Serac codevelop.

CMake Error at cmake/blt/cmake/BLTInstallableMacros.cmake:529 (target_link_libraries):
  Cannot specify link libraries for target "adiak::mpi" which is not built by
  this project.
Call Stack (most recent call first):
  axom/src/cmake/thirdparty/SetupAxomThirdParty.cmake:211 (blt_patch_target)
  axom/src/cmake/CMakeBasics.cmake:20 (include)
  axom/src/CMakeLists.txt:125 (include)

This is the command that fails (within blt's blt_patch_target):

target_link_libraries(adiak::mpi INTERFACE dl)

Within Adiak's section in Axom's setup third party, there is the following:

    get_target_property(_target_type adiak::adiak TYPE)
    if(MPI_FOUND AND ${_target_type} STREQUAL "STATIC_LIBRARY" AND TARGET adiak::mpi)
        blt_patch_target(NAME adiak::mpi DEPENDS_ON dl)
    endif()

I was looking at the adiak::adiak target, and it seems like it contains the dl target. The adiak::mpi target does not.

-- [adiak::adiak property] INTERFACE_LINK_LIBRARIES: dl;adiak::mpi
...
-- [adiak::mpi property] INTERFACE_LINK_LIBRARIES: /usr/tce/packages/mvapich2/mvapich2-2.3.7-clang-14.0.6/lib/libmpi.so;/usr/tce/packages/mvapich2/mvapich2-2.3.7-clang-14.0.6/lib/libmpicxx.so

I am not entirely sure what the issue is, but I figured I'd start by tracking it here.

chapman39 avatar Apr 17 '24 22:04 chapman39

Now that I think about it, Axom may need a check to see if an adiak::adiak target already exists, since that's true if Serac is codeveloping with Axom:

if(TARGET adiak::adiak)
    message(STATUS "Adiak support is ON, using existing adiak target")
    set(ADIAK_FOUND TRUE)
elseif(ADIAK_DIR)
...

chapman39 avatar Apr 17 '24 23:04 chapman39

Alternatively, we can test whether the patch is even needed, since removing blt_patch_target appears to resolve the issue as well.

chapman39 avatar Apr 17 '24 23:04 chapman39

Thanks @chapman39

I'm pretty sure our uberenv setup is only currently testing a SHARED build of adiak (and caliper), so it likely didn't trigger this conditional. If you have a STATIC build and it works without the blt_patch_target, I'd think it would be ok to remove the patch.

As an aside, since we appear to have special logic for STATIC builds, we might consider building at least of of our toss4 configs with adiak~shared and caliper~shared. Perhaps the gcc one?

kennyweiss avatar Apr 18 '24 02:04 kennyweiss

A bit more context:

get_target_property(_target_type adiak::adiak TYPE) if(MPI_FOUND AND ${_target_type} STREQUAL "STATIC_LIBRARY" AND TARGET adiak::mpi) blt_patch_target(NAME adiak::mpi DEPENDS_ON dl) endif()

My adiak branch started at least a year ago, possibly two (see #142), so it's possible this was an artifact from a much older version of adiak that we no longer need to support.

kennyweiss avatar Apr 18 '24 02:04 kennyweiss

I believe Serac is using a static build of adiak, so that would make sense.

chapman39 avatar Apr 18 '24 20:04 chapman39