cmake inconsistently overriding `-O3` (sometimes)

fanquake commented at 2:28 pm on December 13, 2024: member

Using master @ d73f37dda221835b5109ede1b84db2dc7c4b74a1.

The following seems incorrect, because

0make -C depends/ NO_QT=1 NO_WALLET=1 NO_ZMQ=1 -j18 CFLAGS="-O3" CXXFLAGS="-O3"
1cmake -B build --toolchain /root/ci_scratch/depends/aarch64-unknown-linux-gnu/toolchain.cmake
2<snip>
3C++ compiler flags .................... -O3 -O2 -g
4Linker flags .......................... -O3 -O2 -g

is overriding the user-provided -O3.

If you configure by providing an explicit -DCMAKE_BUILD_TYPE=RelWithDebInfo, you get:

0cmake -B build --toolchain /root/ci_scratch/depends/aarch64-unknown-linux-gnu/toolchain.cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo
1C++ compiler flags .................... -O3
2Linker flags .......................... -O3

No -O2 at all, so the user provided -O3 is used (also missing -g for debug info?).

If you configure with -DCMAKE_BUILD_TYPE=Release:

0cmake -B build --toolchain /root/ci_scratch/depends/aarch64-unknown-linux-gnu/toolchain.cmake -DCMAKE_BUILD_TYPE=Release
1C++ compiler flags .................... -O3 -O2
2Linker flags .......................... -O3 -O2

Now we are back to (inconsistently) overriding the user.

fanquake added this to the milestone 29.0 on Dec 13, 2024

hebasto commented at 2:47 pm on December 13, 2024: member

Optimization levels and debugging options are tied to a “build type/configuration” in CMake. If a user wants to override any of these, they should use the CMAKE_CXX_FLAGS_<CONFIG> variables. In depends, the $(host)_{release,debug}_CXXFLAGS variables handle this. However, they are not overridable by the user at the moment.

fanquake commented at 3:13 pm on December 13, 2024: member

If a user wants to override any of these, they should use the CMAKE_CXX_FLAGS_ variables.

Any user would expect this to be handled by the toolchain file, which is meant to pass everything required through to CMake. Regardless of that, the fact that the behaviour here is inconsistent depending on whether or not you pass -DCMAKE_BUILD_TYPE=RelWithDebug explictly, is confusing.

fanquake commented at 3:21 pm on December 13, 2024: member

If a user wants to override any of these, they should use the CMAKE_CXX_FLAGS_ variables.

Does this actually work in this case though? If I do:

0cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_FLAGS_RELEASE="-O3"
1<snip>
2C++ compiler flags .................... -O2 -std=c++20 -fPIC -fno-extended-identifiers -fdebug-prefix-map=/root/ci_scratch/src=. -fmacro-prefix-map=/root/ci_scratch/src=. -fstack-reuse=none -Wall -Wextra -Wformat -Wformat-security -Wvla -Wredundant-decls -Wdate-time -Wduplicated-branches -Wduplicated-cond -Wlogical-op -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wbidi-chars=any -Wundef -Wno-unused-parameter -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wstack-protector -fstack-protector-all -fstack-clash-protection -mbranch-protection=standard

I still get -O2 ?

theuni commented at 7:34 pm on December 13, 2024: member

I still get -O2 ?

I’m seeing the same. This definitely worked at some point, I assume there’s been some regression since merge?

theuni commented at 7:37 pm on December 13, 2024: member

Ah, I see. It’s: replace_cxx_flag_in_config(Release -O3 -O2)

I suppose that should only happen if the -O3 isn’t coming from an explicitly set CMAKE_CXX_FLAGS_RELEASE.

fanquake commented at 12:09 pm on December 16, 2024: member

I suppose that should only happen if the -O3 isn’t coming from an explicitly set CMAKE_CXX_FLAGS_RELEASE.

Possibly, but I think the way this has been implented is confusing (also not properly documented), and the goals are unclear. Are we trying to prevent compilation with -O3, or just trying to make things as non-standard (CMake) as possible. Why even do the replacement if you are going to let it be overridden (using the standard flags)? The current code, and that behaviour are just going to trip up everyone that expects ‘Release’ to already be a certain thing.

cmake inconsistently overriding -O3 (sometimes) #31491

cmake inconsistently overriding `-O3` (sometimes) #31491