Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Readme update #659

Merged
merged 4 commits into from
Jul 5, 2022
Merged

Readme update #659

merged 4 commits into from
Jul 5, 2022

Conversation

gareth-nx
Copy link
Contributor

This pull request explains some CMake options that affect stdlib builds.

The first issue is that user-specified compiler optimizations in FFLAGS are overridden by the default the stdlib CMake setup. This means that, in the original README, the use of FFLAGS was not having any effect.

  • To fix this, the updated README discusses -DCMAKE_BUILD_TYPE. This can be setup to avoid default optimizations (while retaining flags that are essential to compile stdlib). This allows optimization flags in FFLAGS to be used.

The second issue is that by default, the user does not see what commands CMake is using to compile the code. This makes it difficult to notice problems like those above.

  • The solution is to set -DCMAKE_VERBOSE_MAKEFILE=On, which prints the compiler options.

I am a CMake novice, so it would be good to have this checked by an expert like @awvwgk .

Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look good to me, thanks!

@awvwgk awvwgk added documentation Improvements or additions to documentation build: cmake Issue with stdlib's CMake build files labels Jun 13, 2022
@awvwgk
Copy link
Member

awvwgk commented Jun 13, 2022

What happens if you set -DCMAKE_BUILD_TYPE=Release instead of using NoConfig and setting FFLAGS manually?

@gareth-nx
Copy link
Contributor Author

What happens if you set -DCMAKE_BUILD_TYPE=Release instead of using NoConfig and setting FFLAGS manually?

It seems to build without any optimizations.

To be specific, I delete the build and installation directories, then run the following from inside the directory /home/gareth/Code_Experiments/fortran/stdlib/my_branch/stdlib/

export FC="gfortran"
cmake -B build -DCMAKE_MAXIMUM_RANK=7 -DCMAKE_INSTALL_PREFIX=/home/gareth/Code_Experiments/fortran/stdlib/my_branch/stdlib/local_install_gfortran/ -DCMAKE_VERBOSE_MAKEFILE=On -DCMAKE_BUILD_TYPE=Release

Then the printed compiler commands in the build process look like this example -- (no optimization flags)

/usr/bin/gfortran  -I/home/gareth/Code_Experiments/fortran/stdlib/my_branch/stdlib/build/src/mod_files  -fimplicit-none -ffree-line-length-132 -Jmod_files/ -fPIC   -fno-range-check -c /home/gareth/Code_Experiments/fortran/stdlib/my_branch/stdlib/build/src/stdlib_hash_64bit_fnv.f90 -o CMakeFiles/fortran_stdlib.dir/stdlib_hash_64bit_fnv.f90.o

Incidentally I believe that this behaviour has changed since last year sometime (when Release led to -O3 type optimization). Back then, it also had the shortcoming of overriding the FFLAGS optimizations.

Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds good to me. IMO it could be merged, pending @awvwgk 's comments.

@awvwgk
Copy link
Member

awvwgk commented Jun 18, 2022

The Release flags are currently empty

set(
CMAKE_Fortran_FLAGS_RELEASE_INIT
)

You could add the preferred options there and we can rely on -DCMAKE_BUILD_TYPE=Release rather than reverting to the NoConfig which is the usual CMake default.

@jvdp1
Copy link
Member

jvdp1 commented Jun 19, 2022

The Release flags are currently empty

set(
CMAKE_Fortran_FLAGS_RELEASE_INIT
)

You could add the preferred options there and we can rely on -DCMAKE_BUILD_TYPE=Release rather than reverting to the NoConfig which is the usual CMake default.

I would propose to use the same compiler options as for the release option of fpm:

 -O3 -funroll-loops -Wimplicit-interface -fPIC -fmax-errors=1 -fcoarray=single

@jvdp1
Copy link
Member

jvdp1 commented Jul 4, 2022

@awvwgk Are you good with these changes? Can this PR be merged?

@jvdp1
Copy link
Member

jvdp1 commented Jul 5, 2022

Thank you all. I will merge it.

@jvdp1 jvdp1 merged commit 2779aa8 into fortran-lang:master Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build: cmake Issue with stdlib's CMake build files documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants