-
Notifications
You must be signed in to change notification settings - Fork 192
Description
Description
README.md instructs users to control compiler flags in CMake builds like this (e.g. to get "-O3"):
export FFLAGS="-O3"
cmake -B build -G Ninja -DCMAKE_MAXIMUM_RANK:String=7 -DCMAKE_INSTALL_PREFIX=$HOME/.local
However this does not have the intended effect; by default there are other options that override this optimization.
To see that this is the case, we can insert the line set( CMAKE_VERBOSE_MAKEFILE on )
near the top of CMakeLists.txt
, and build with the above commands. Then the compiler commands are printed out. Copying one (for illustration) we have:
/usr/bin/gfortran -I/home/gareth/Code_Experiments/fortran/stdlib/my_branch/stdlib/build/src/mod_files -O3 -O2 -g -DNDEBUG -Jmod_files/ -fimplicit-none -ffree-line-length-132 -Wall -Wextra -Wimplicit-procedure -pedantic-errors -std=f2018 -c /home/gareth/Code_Experiments/fortran/stdlib/my_branch/stdlib/build/src/stdlib_bitsets_64.f90 -o CMakeFiles/fortran_stdlib.dir/stdlib_bitsets_64.f90.o
Notice how right after the "-O3"
, we have "-O2 -g -DNDEBUG"
. This is overriding the "-O3"
optimization which was intended. Our documentation doesn't make this clear, and this has confused some efforts to try optimising code.
Expected Behaviour
I expected the code to have been compiled with -O3
, not -O2
.
The reason I noticed this is that my code was slower when linking to Cmake builds, vs Makefile builds (and hence I was getting weird performance results).
Version of stdlib
Platform and Architecture
All
Additional Information
It seems that by default, stdlib
is having CMake add the flags for a CMAKE_BUILD_TYPE=RelWithDebInfo
build.
One way to work-around this issue is to have the user make up some ad-hoc BUILD_TYPE (so that the defaults are not invoked). However presumably there are some options that we really do want by default -- so the questions are:
- What should we suggest in README.md?
- Should any of the CMake defaults be changed?
I think it's a good idea to use CMAKE_VERBOSE_MAKEFILE=On (or suggest that option to users in the documentation). This prints out the compiler commands (so users will likely notice if they differ from what was intended).
export FFLAGS="-O3 -fno-range-check"
cmake -B build -DCMAKE_MAXIMUM_RANK=7 -DCMAKE_INSTALL_PREFIX=/home/gareth/Code_Experiments/fortran/stdlib/my_branch/stdlib/local_install_gfortran/ -DCMAKE_BUILD_TYPE=MyCustomBuild -DCMAKE_VERBOSE_MAKEFILE=On
Activity
diff
procedure #606awvwgk commentedon Jan 3, 2022
Optimization levels in CMake are mainly controlled by the
-DCMAKE_BUILD_TYPE
, we default toNoConfig
, which takes whatever CMake thinks is the best default. For most projects I preferRelWithDebInfo
for development andRelease
build types for profiling. We should mention the possibility to adjust this via the standard CMake option.gareth-nx commentedon Jan 3, 2022
How should we recommend that users set
-DCMAKE_BUILD_TYPE
?If they use
-DCMAKE_BUILD_TYPE=Release
, then I believe that will append compiler flags that include-O3
. This suffers the same problem that it can override otherFFLAGS
(e.g. say-Ofast
was specified, it would be silently overridden). So IMO it isn't the right solution (unless further qualified).We could tell users to set
-DCMAKE_BUILD_TYPE=somethingWithoutADefault
if they want to prevent their compiler flags being overridden. But it could be prone to other problems (e.g. gfortran-9 users better specify-fno-range-check
to avoid problems with the hash routines).awvwgk commentedon Jan 3, 2022
We could provide an overwrite file like in LFortran to purge unwanted arguments from the profiles, or at least the default profile.
gareth-nx commentedon Jan 3, 2022
That sounds good.
So in the README, I imagine that, rather than explaining
FFLAGS
, we would say:-DCMAKE_BUILD_TYPE
when compiling stdlib (common values are-DCMAKE_BUILD_TYPE=Release
for builds with high optimization, and-DCMAKE_BUILD_TYPE=Debug
for debugging builds).FYI I'm traveling from tomorrow until late January (without a computer), but will follow up in a few weeks if it hasn't been addressed already.
awvwgk commentedon Jan 3, 2022
I don't really want to recommend users to overwrite checked in files in our repo. Instead we should provide a way to just use
FFLAGS
or have users set their arguments via-DCMAKE_Fortran_FLAGS="$FFLAGS"
which will overwrite the CMake defaults. We could implement some logic to always useFFLAGS
to overwrite the respectiveCMAKE_Fortran_FLAGS_*
variables for all build-types as well.I might look into this, if I find some motivation to write needlessly trivial but lengthy stuff in CMake, because it is not available by default.
And, of course, have a nice travel.