HiGHS icon indicating copy to clipboard operation
HiGHS copied to clipboard

CMakeLists.txt RELWITHDEBINFO handling joins RELEASE and DEBUG flags causing msvc conflict /RTC and /O

Open Thell opened this issue 2 years ago • 2 comments

https://github.com/ERGO-Code/HiGHS/blob/d4809eae74a89e4bb36dd45dca2c7e08c14d5ef1/CMakeLists.txt#L187

image

I had originally opened this to the highs-sys project (https://github.com/rust-or/highs-sys/issues/11) but have now found where it comes from (linked above).

CMAKE_CXX_FLAGS_DEBUG:STRING=/Zi /Ob0 /Od /RTC1 CMAKE_CXX_FLAGS_RELEASE:STRING=/O2 /Ob2 /DNDEBUG

This is what it should be according to CMakeCache.txt -> CMAKE_CXX_FLAGS_RELWITHDEBINFO:STRING=/Zi /O2 /Ob1 /DNDEBUG when configured with Ninja using

running: "cmake" "C:\\Users\\thell\\.cargo\\git\\checkouts\\highs-sys-libz-228d8310235e4834\\acefa53\\HiGHS" "-G" "Ninja" "-DFAST_BUILD=ON" "-DSHARED=OFF" "-DCMAKE_INSTALL_PREFIX=C:\\Users\\thell\\Workspaces\\bdo\\housecraft\\target\\release\\build\\highs-sys-6d2879004261603f\\out" "-DCMAKE_C_FLAGS= -nologo -MD -Brepro" "-DCMAKE_C_COMPILER=C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.35.32215/bin/HostX64/x64/cl.exe" "-DCMAKE_CXX_FLAGS= -nologo -MD -Brepro" "-DCMAKE_CXX_COMPILER=C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.35.32215/bin/HostX64/x64/cl.exe" "-DCMAKE_ASM_FLAGS= -nologo -MD -Brepro" "-DCMAKE_ASM_COMPILER=C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.35.32215/bin/HostX64/x64/cl.exe" "-DCMAKE_BUILD_TYPE=RelWithDebInfo"

but we end up with both /RTC1 and /O2 because of the direct merging which fails because according to MSDN it isn't allowed.

If you compile your program at the command line using any of the /RTC compiler options, any pragma optimize instructions in your code silently fail. That's because run-time error checks aren't valid in a release (optimized) build. Use /RTC for development builds; Don't use /RTC for a release build. /RTC can't be used with compiler optimizations (/O Options (Optimize Code)). A program image built with /RTC is slightly larger and slightly slower than an image built with /Od (up to 5 percent slower than an /Od build).

Thell avatar Apr 25 '23 04:04 Thell

We may be able to accommodate this - what's your view @galabovaa? - but why do you want to build HiGHS in anything other than Release?

jajhall avatar Apr 25 '23 13:04 jajhall

why do you want to build HiGHS in anything other than Release?

I want to build HiGHS in Release but I want to build my projects in whichever I want and the highs-sys crate uses the cmake crate to automate the toolchain and that cmake crate automates the config and building of HiGHS by setting up the environment (just we all expect toolchains to do).

The problem is when the CMake crate passes along a project's settings and those conflict with HiGHS. Seriously, without modifying the high-sys crate you can't build a Release build of HiGHS on Windows even with

    let dst = Config::new("HiGHS")
        .define("FAST_BUILD", "ON")
        .define("SHARED", "OFF")
        .define("CMAKE_MSVC_RUNTIME_LIBRARY", "MultiThreadedDLL")
        .define("CMAKE_INTERPROCEDURAL_OPTIMIZATION", "FALSE")
        .build();

which should build Release. And, well, sure enough it is technically a Release build but without passing NDEBUG which means someone like me ends up hitting asserts even when they expect to be in release and the only real reason for that to happen is if the developers explicitly make it happen which means opening tickets with lots of noise with low signal which no-one likes. The only way I could get a Release build while using the highs-sys -> cmake crate was to specify Ninja as the generator.

This is all to say I wanted and want nothing to do with HiGHS builds but here we are.

Perhaps getting high-sys to use feature flags to indicate the build, which makes sense for a 3rd party dependency, would be nice. I'll look at making that a reality.

Thell avatar Apr 25 '23 16:04 Thell

Hi, apologies for the delay, we are now going to look into this. Can you please confirm that the issue is still present in our latest version 1.7.0 in master? My guess is that it would be. I would try to remove the conflicting lines in the meantime.

Did you find a way to do it one the highs-sys side?

galabovaa avatar Mar 11 '24 15:03 galabovaa

I have just removed the lines causing the issue, this will be included in our next release. In the meantime, it would be great if you could double check that this is no longer an issue with our "latest" branch!

galabovaa avatar Mar 12 '24 00:03 galabovaa

Confirmed to be working. Fixed in highs-sys https://github.com/rust-or/highs-sys/issues/11

Thell avatar Apr 04 '24 00:04 Thell