Conversation
Grufoony
commented
Dec 23, 2025
- Close Avoid arch specific flags when compiling for pypi #386
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #387 +/- ##
=======================================
Coverage 83.54% 83.54%
=======================================
Files 53 53
Lines 5401 5401
Branches 618 618
=======================================
Hits 4512 4512
Misses 878 878
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #386 by preventing the use of native architecture optimizations (-march=native) when building Python wheels for PyPI distribution. The change ensures that built wheels are compatible with a wider range of CPU architectures rather than being optimized only for the build machine's specific CPU.
Key changes:
- Introduced a new CMake option
DSF_OPTIMIZE_ARCH(default: ON) to control architecture-specific optimizations - Modified build flags to conditionally apply
-march=nativeonly when the option is enabled - Updated PyPI build workflows to explicitly disable architecture optimization via
CMAKE_ARGS
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/dsf/dsf.hpp | Bumped patch version from 3 to 4 to reflect the new release |
| CMakeLists.txt | Added DSF_OPTIMIZE_ARCH option and made -march=native flag conditional in Release and Profile build modes |
| .github/workflows/pypi.yml | Set CMAKE_ARGS: "-DDSF_OPTIMIZE_ARCH=OFF" for all three platform build jobs (Linux, macOS, Windows) to ensure portable wheels |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CMakeLists.txt
Outdated
| if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Ofast -march=native -flto=auto") | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Ofast -flto=auto") | ||
| if (DSF_OPTIMIZE_ARCH) |
There was a problem hiding this comment.
The spacing in the if statement is inconsistent with the rest of the file. The file consistently uses if( without a space between if and the opening parenthesis, but this line uses if ( with a space. Please change to if(DSF_OPTIMIZE_ARCH) to match the project's coding style.
CMakeLists.txt
Outdated
| if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Ofast -march=native -flto=auto -pg") | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Ofast -flto=auto -pg") | ||
| if (DSF_OPTIMIZE_ARCH) |
There was a problem hiding this comment.
The spacing in the if statement is inconsistent with the rest of the file. The file consistently uses if( without a space between if and the opening parenthesis, but this line uses if ( with a space. Please change to if(DSF_OPTIMIZE_ARCH) to match the project's coding style.