Skip to content

Fix SIMD compilation error

Jacques Xing requested to merge CFD-Xing/nektar:disable_simd_sse2 into master

Issue/feature addressed

The CMAKE_CXX_FLAGS compilation flags for SIMD are not set properly. This MR fix this issue.

For some reasons the CMAKE_CXX_FLAGS variable is not updated if the CACHE STRING option is used, unless combined with the FORCE option. This can be confirmed by adding a MESSAGE(CMAKE_CXX_FLAGS = ${CMAKE_CXX_FLAGS}) command in NektarSIMD.cmake to print out the value of the variable. The consequence of that is the compiler options for AVX2 (-mavx2 -mfma) are not automatically set when running cmake.

If one does not use the CACHE STRING option, the CMAKE_CXX_FLAGS variable is updated properly but this does not show up in the ccmake gui. However, doing so overwrite the CMAKE_CXX_FLAGS variable and the compiler flag is only set for SSE2 (-msse2) since SSE2 is automatically activated along with AVX2.

Proposed solution

The solution proposed is to add cached advanced variable for the compilation flags for AVX512, AVX2, and SSE2. Then, those flags are appended internally to CMAKE_CXX_FLAGS such that the compilation flag are set properly although they will not show up in the ccmake for the CMAKE_CXX_FLAGS variable, but only individually

SIMD

Also, the CMAKE_CXX_FLAGS variable should NOT be cached along with appending, doing so will recursively append the SIMD compilation flags everything the command cmake.. is run. The current implementation also automatically removed the relevant SIMD compilation flag is SIMD is disable.

Implementation

Tests

Suggested reviewers

Please suggest any people who would be appropriate to review your code.

Notes

Please add any other information that could be useful for reviewers.

Checklist

  • Functions and classes, or changes to them, are documented.
  • [ ] User guide/documentation is updated.
  • Changelog is updated.
  • [ ] Suitable tests added for new functionality.
  • Contributed code is correctly formatted. (See the contributing guidelines).
  • [ ] License added to any new files.
  • No extraneous files have been added (e.g. compiler output or test data files).
Edited by Jacques Xing

Merge request reports