Add a conditional disable of passing openmp flags on mac with clang #165
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds conditional compilation logic to disable OpenMP compiler flags when building on macOS with AppleClang, addressing build issues on Mac systems. The author notes this is a temporary workaround that should be properly resolved in the future.
Key Changes
- Added conditional checks to skip OpenMP compile options on macOS with AppleClang compiler
- Added TODO comments noting this is a temporary solution pending a proper fix
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pyEXP/CMakeLists.txt | Added conditional logic to skip OpenMP flags for AppleClang on macOS when building the pyEXP module |
| expui/CMakeLists.txt | Added identical conditional logic to skip OpenMP flags for AppleClang on macOS when building expui executables |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| target_compile_options(pyEXP PUBLIC ${OpenMP_CXX_FLAGS}) | ||
| # TODO: Should find a proper solution for this, but for now, don't add OpenMP compile | ||
| # options if compiling on macOS with Clang | ||
| if(NOT (APPLE AND CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")) |
There was a problem hiding this comment.
Consider checking if OpenMP was actually found before applying flags. The condition should verify OpenMP_CXX_FOUND to ensure OpenMP is available on the system. This prevents attempting to use undefined ${OpenMP_CXX_FLAGS} on systems without OpenMP support.
| if(NOT (APPLE AND CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")) | |
| if(OpenMP_CXX_FOUND AND NOT (APPLE AND CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")) |
There was a problem hiding this comment.
Our flag is OpenMP_FOUND as far as I can tell; this could be a possible change but I'm not sure if we need to.
2 participants
I have long been commenting out these lines in order to build locally on mac with clang, but here is a simple conditional to do it for me/us.
In the long run, we should figure out why this is necessary on Mac and whether we can get it working.