ekg / seqwish

alignment to variation graph inducer
MIT License
143 stars 18 forks source link

Issue #104 - Use -march=native as default #106

Closed martin-g closed 2 years ago

martin-g commented 2 years ago

Make it possible to overwrite it with -DEXTRA_FLAGS="-march=armv8-a"

Related-to: #104

martin-g commented 2 years ago

// CC @AndreaGuarracino

AndreaGuarracino commented 2 years ago

@martin-g, I apologize if I am misunderstanding your commits, but now -march=native would be set also in Generic mode, a mode that should be much general as possible, that is, it should avoid too heavy optimizations.

Indeed, now if I put message(STATUS "EXTRA_FLAGS: ${EXTRA_FLAGS}") in the CMakeLists.txt file, I get

cmake -H. -Bbuild -DCMAKE_BUILD_TYPE=Generic && cmake --build build -- -j 2

-- CMAKE_BUILD_TYPE: Generic
-- CMAKE_SYSTEM_NAME: Linux
-- EXTRA_FLAGS: -march=native
martin-g commented 2 years ago

@AndreaGuarracino My idea in the PR is to set native as default and the user can do:

cmake -H. -Bbuild -DCMAKE_BUILD_TYPE=Generic -DEXTRA_FLAGS='-march=haswell' && cmake --build build -- -j 2

or

cmake -H. -Bbuild -DCMAKE_BUILD_TYPE=Generic -DEXTRA_FLAGS='-march=armv8-a' && cmake --build build -- -j 2

or any other valid -march value for his/her machine.

I am suggesting this PR because at the moment build type Generic is unusable for Linux ARM64 users due to the hardcoded haswell value.

I am not aware of cross platform way to detect the CPU architecture in CMakeLists.txt to do if/elsif/else for all possible combinations of build types and marches.

julien-faye commented 2 years ago

It is not just Generic build type. ARM64 users cannot use also Debug build type due to haswell. I like how this PR makes it possible to set the -march to a custom value!

AndreaGuarracino commented 2 years ago

Ok, there seems to be no single general solution to fix all possible scenarios. I thought that for ARM solutions it was enough to compile in Release mode which triggers -march=native.

I like not hardcoding haswell, and I like giving the user the way to rule the compilation without sed tricks to the CMakeLists.txt. I will document these new options and update pggb accordingly. Thank you @martin-g and @julien-faye for your PRs and feedback!