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

[DOC] cmake default without -O3 and order of args/options in cmake call #2017

Open
marehr opened this issue Aug 10, 2020 · 1 comment
Open

[DOC] cmake default without -O3 and order of args/options in cmake call #2017

marehr opened this issue Aug 10, 2020 · 1 comment

Comments

@marehr
Copy link
Member

@marehr marehr commented Aug 10, 2020

Stefan Kurtz reported the following issue over our seqan-dev list:

  1. I followed the description for the Cmake-setup and noticed in the example CMakeList.txt shown at the end of https://docs.seqan.de/seqan/3-master-user/setup.html that compiler optimization options such as -O3 is missing, which will lead to poor performance for users who do not recognize it.

    I now use

    add_compile_options(-DNDEBUG -Wall -Wextra -Wunused-parameter -Wunused-variable -pedantic -Werror -O3)

    in CmakeLists.txt.

  2. The cmake-setup specifies that one has to call
    cmake ../source -DCMAKE_CXX_COMPILER=/path/to/executable/g++-7

    The help pages of cmake version 3.16.3 on macOS and cmake version 3.10.2 on Linux state that options come before arguments, but only the macOS version enforces this. So on my macOS system only

    cmake  -DCMAKE_CXX_COMPILER=/path/to/executable/g++-7 ../source

    works. To simplify the use for macOS, it is probably better use this order in the cmake-setup section (and elsewhere) which works on both platforms.

Platform

  • SeqAn version:
  • Operating system:
  • Compiler: <!-- enter output of /path/to/compiler --version here -->

Description

How to repeat the problem

Expected behaviour

Actual behaviour

@marehr
Copy link
Member Author

@marehr marehr commented Aug 10, 2020

1. I followed the description for the Cmake-setup and noticed in the example CMakeList.txt shown at the end of https://docs.seqan.de/seqan/3-master-user/setup.html that compiler optimization options such as -O3 is missing, which will lead to poor performance for users who do not recognize it.
   I now use
   ```cmake
   add_compile_options(-DNDEBUG -Wall -Wextra -Wunused-parameter -Wunused-variable -pedantic -Werror -O3)
   ```
   
   
   in CmakeLists.txt.

Thank you for pointing this out. It is a known fact / best practice in the CMake community that -DCMAKE_BUILD_TYPE=Release must be specified to enable optimized builds, e.g. cmake -DCMAKE_BUILD_TYPE=Release <seqan3-source-path> , but since our documentations should be accessible for "new" C++ programmers or "new" CMake users, we should add a section that teaches how to enable Release builds.

The best-practice in CMake for header-only libraries, like SeqAn2 or SeqAn3, is to specify only the minimal required flags that are needed to compile any given source that uses SeqAn. Additional flags, like hardening options or performance options is up to the application developer and/or package maintainer. The rational is simple, if we would add the -O3 flag to our cmake export, no-one could do a debug-build any more, because we then would dictate you to always have the -O3 flag in your Makefile.

2\. The cmake-setup specifies that one has to call
    cmake ../source -DCMAKE_CXX_COMPILER=/path/to/executable/g++-7
    The help pages of cmake version 3.16.3 on macOS and cmake version 3.10.2 on Linux state that options come before arguments, but only the macOS version enforces this. So on my macOS system only
    ```shell
    cmake  -DCMAKE_CXX_COMPILER=/path/to/executable/g++-7 ../source
    ```
    
    
    works. To simplify the use for macOS, it is probably better use this order in the cmake-setup section (and elsewhere) which works on both platforms.

Thank you for this, too :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.