Conversation
|
Thanks for opening this pull request! Please check out our contributing guidelines. |
amametjanov
added
cmake
There was a problem hiding this comment.
Pull request overview
This PR updates EKAT’s CMake compiler-flag setup to avoid passing the Intel classic -restrict C++ flag to IntelLLVM/oneAPI toolchains (e.g., icpx), addressing EKAT#412 by eliminating related warnings in oneAPI builds.
Changes:
- Restricts use of
-restrictto theIntel(classic) Fortran toolchain path. - Splits
IntelvsIntelLLVMhandling inSetGeneralFlagsso IntelLLVM no longer receives-restrictviaCXXFLAGS.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (CMAKE_Fortran_COMPILER_ID STREQUAL "Intel") | ||
| SetFlags (FFLAGS "-assume byterecl -ftz" CXXFLAGS -restrict) | ||
| elseif (CMAKE_Fortran_COMPILER_ID STREQUAL "IntelLLVM") | ||
| SetFlags (FFLAGS "-assume byterecl -ftz") | ||
| elseif (CMAKE_Fortran_COMPILER_ID STREQUAL "GNU") | ||
| SetFlags (FFLAGS -ffree-line-length-none) | ||
| endif() |
There was a problem hiding this comment.
-restrict is a C/C++-only flag, but it’s currently being added based on CMAKE_Fortran_COMPILER_ID. The project explicitly supports using different compilers per language, so this can still pass -restrict to a non-Intel C++ compiler (e.g., icpx/IntelLLVM or GCC) when Fortran is Intel, reintroducing the warning/flag error.
Consider splitting the call: always set the Intel Fortran flags, and add CXXFLAGS -restrict only when CMAKE_CXX_COMPILER_ID STREQUAL "Intel" (and not for IntelLLVM).
| if (CMAKE_Fortran_COMPILER_ID STREQUAL "Intel") | |
| SetFlags (FFLAGS "-assume byterecl -ftz" CXXFLAGS -restrict) | |
| elseif (CMAKE_Fortran_COMPILER_ID STREQUAL "IntelLLVM") | |
| SetFlags (FFLAGS "-assume byterecl -ftz") | |
| elseif (CMAKE_Fortran_COMPILER_ID STREQUAL "GNU") | |
| SetFlags (FFLAGS -ffree-line-length-none) | |
| endif() | |
| # Fortran compiler-specific flags | |
| if (CMAKE_Fortran_COMPILER_ID STREQUAL "Intel") | |
| SetFlags (FFLAGS "-assume byterecl -ftz") | |
| elseif (CMAKE_Fortran_COMPILER_ID STREQUAL "IntelLLVM") | |
| SetFlags (FFLAGS "-assume byterecl -ftz") | |
| elseif (CMAKE_Fortran_COMPILER_ID STREQUAL "GNU") | |
| SetFlags (FFLAGS -ffree-line-length-none) | |
| endif() | |
| # C++ compiler-specific flags | |
| if (CMAKE_CXX_COMPILER_ID STREQUAL "Intel") | |
| SetFlags (CXXFLAGS -restrict) | |
| endif() |
There was a problem hiding this comment.
Looks apt for IntelLLVM and Intel compiler toolchains!
There was a problem hiding this comment.
@bartgol do you want me to make the changes suggested by copilot?
There was a problem hiding this comment.
Yeah, I think that's a good idea. We don't want to get lots of compiler warnings b/c the ftn flag is not recognized...
There was a problem hiding this comment.
Ok, applied the suggestion.
Please feel free to merge or let me know if i should merge.
bartgol
left a comment
bartgol
left a comment
There was a problem hiding this comment.
Thanks!
We should prob start to go through ekat and eamxx, to ensure we set "generic" flags only if building in standalone mode (and not as part of e3sm).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Congrats on merging your first pull request! We hope this is only the first of many to come. |
Remove
-restrictfrom icpx CXXFLAGS.Fixes #412
[BFB]
Testing:
-restrict