HiGHS icon indicating copy to clipboard operation
HiGHS copied to clipboard

Error when linking to HiGHS with clang15

Open FlorianSchwendinger opened this issue 2 years ago • 5 comments

In the new HiGHS version LTO is enabled by default for LINUX and clang. The corresponding code chunk from the CMakeList.txt file can be seen below:

if(LINUX AND NOT MSVC)
    include(CheckIPOSupported)
    check_ipo_supported(RESULT ipo_supported OUTPUT error)

    if(ipo_supported)
        message(STATUS "IPO / LTO enabled")
        set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
    else()
        message(STATUS "IPO / LTO not supported: <${error}>")
    endif()
endif()

When I try to link to the HiGHS library with clang15 on debian testing I get the following error message.

/usr/bin/ld: /tmp/Rtmpy6uejk/R.INSTALL3d922bc7e7f1/highs/src/highslib/lib/libhighs.a: error adding symbols: file format not recognized
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I resolved it in my installation by changing

if(LINUX AND NOT MSVC)

to

if(LINUX AND (NOT MSVC) AND (NOT CMAKE_CXX_COMPILER_ID STREQUAL "Clang"))

Not sure what is the best fix HiGHS, since I am not sure if this is a debian testing issue or a clang15 issue.

FlorianSchwendinger avatar Dec 10 '22 11:12 FlorianSchwendinger

@FlorianSchwendinger You are compiling with clang but linking with the GNU linker, ld. LTO is compiler-specific, so the GNU linker can only perform LTO on the output of GCC, not on the output of clang. cmake -DCMAKE_CXX_FLAGS="-fuse-ld=lld" tells CMake to tell the compiler to use clang's lld linker instead.

The underlying problem in HiGHS is that LTO (IPO in CMake terminology) is forcibly enabled if supported by the compiler, even if the user turned it off explicitly using -DCMAKE_INTERPROCEDURAL_OPTIMIZATION=OFF. If CMAKE_INTERPROCEDURAL_OPTIMIZATION already has a value, I think CMakeLists.txt should not override it.

Also consider enabling LTO by default only if HiGHS is compiled as a dynamic library. When building as a static library, HiGHS shouldn't assume that it will eventually get linked with a compatible linker, of a compatible version, with compatible settings.

ttencate avatar Dec 21 '22 12:12 ttencate

The above commit fixes it for me. I can send a PR if desired.

ttencate avatar Dec 21 '22 14:12 ttencate

@jajhall @galabovaa , can someone please merge this fix ?

lovasoa avatar Jan 17 '23 23:01 lovasoa

The above commit fixes it for me. I can send a PR if desired.

@ttencate Please would you send a PR (to the HiGHS branch latest, not master) for the commit with the fix.

jajhall avatar Jan 17 '23 23:01 jajhall

@jajhall #1103 is a good step, but I'll propose something slightly more ambitious, hang on...

ttencate avatar Jan 18 '23 09:01 ttencate

Hi all, apologies for the delay. Am I correct in assuming that you still get this issue with the latest version of HiGHS (1.7.0 from master)? We have now completed the re-structuring of CMake that was required for python, and can look at addressing this usage of LPO on platforms with settings that may or may not support it.

Does the commit referenced above resolve the issue? If so, would you mind opening a PR integrating it into HiGHS? We would ideally like to have the fix integrated and avoid users having to use patches to get it to work. If you are happy to do that, please open the PR to the branch "latest"!

galabovaa avatar Mar 11 '24 15:03 galabovaa

I'm longer actively working with HiGHS and don't have time to do open a PR, but feel free to steal the patch as it is.

ttencate avatar Mar 11 '24 19:03 ttencate

Thank you, I have just added this! Will be merged in latest.

galabovaa avatar Mar 11 '24 23:03 galabovaa