Skip to content

Remove gcc flags when using msvc for building pyposelib #132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 4, 2025
Merged

Remove gcc flags when using msvc for building pyposelib #132

merged 1 commit into from
Jul 4, 2025

Conversation

theartful
Copy link
Contributor

Building the python wheel on windows using msvc fails otherwise.

@pablospe
Copy link
Collaborator

Are all the target_compile_options failing?

target_compile_options(pyposelib PRIVATE -march=native -Wall -Werror -fPIC -Wno-ignored-optimization-argument)

@theartful
Copy link
Contributor Author

@pablospe Windows doesn't have an equivalent to march=native as far as I know. Instead it has specific cpu feature flags. -Wall and -Werror has their equivalents: /Wall and /WX, see https://learn.microsoft.com/en-us/cpp/build/reference/compiler-option-warning-level?view=msvc-170
As for -fPIC there is no equivalent in MSVC.

So I think we're left with /Wall and /WX, and maybe we can check for CPU features and add /arch:SSE2, /arch:AVX, etc.

@theartful
Copy link
Contributor Author

theartful commented Jan 22, 2025 •
edited
Loading

Btw, I didn't bother to further investigate since this same pattern was used here, where PoseLib is built:

PoseLib/CMakeLists.txt

Lines 23 to 40 in 56d158f

# Compilation options
option(MARCH_NATIVE "Enable CPU specific optimizations" OFF)
if(MSVC)
target_compile_options(${LIBRARY_NAME} PRIVATE /bigobj /fp:fast)
else()
# If you change this, make sure to update the corresponding line in the pybind CMakeLists
if (MARCH_NATIVE)
target_compile_options(${LIBRARY_NAME} PRIVATE
-march=native -Wall -Werror -fPIC -Wno-ignored-optimization-argument)
else()
target_compile_options(${LIBRARY_NAME} PRIVATE
-Wall -Werror -fPIC)
endif()
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
target_compile_options(${LIBRARY_NAME} PRIVATE
-Wno-maybe-uninitialized)
endif()
endif()

@pablospe
Copy link
Collaborator

pablospe commented Apr 9, 2025

@vlarsson Should we merge this?

@vlarsson
Copy link
Collaborator

vlarsson commented Jul 4, 2025

LGTM.

@vlarsson vlarsson merged commit 0b57c55 into PoseLib:master Jul 4, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablospe pablospe pablospe approved these changes

@vlarsson vlarsson Awaiting requested review from vlarsson vlarsson is a code owner

@Parskatt Parskatt Awaiting requested review from Parskatt Parskatt is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@theartful @pablospe @vlarsson