Skip to content
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.

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

Fix compilation using default options on Emscripten #470

Merged
merged 1 commit into from
May 17, 2020

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented May 17, 2020

By default, fcl compiles with the FCL_USE_X64_SSE option enabled, that compiles the library
with SSE flags enabled. This creates problems in any compiler that do not targets x86 or x86_64 .

While the problem can be easily solved by user by setting the FCL_USE_X64_SSE to OFF
(as done for example in Debian packaging scripts, see https://salsa.debian.org/science-team/fcl/-/blob/61641fb7a12d19806c799503fc90fcd64b20c5ad/debian/rules#L10) it is nice to provide a working
build out of the box.

Initially I wanted to provide a more general fix, by setting FCL_TARGET_SUPPORT_X64_SSE to ON only if the target architecture is x86 or x86_64, but then I discovered that it is surprisingly difficult to check in CMake the target architecture in a multi-platform way (see https://stackoverflow.com/questions/11944060/how-to-detect-target-architecture-using-cmake), so for now I just did a conservative modification of setting the default value of FCL_USE_X64_SSE to OFF on Emscripten (that was the initial target for which I found this issue).


This change is Reviewable

By default, fcl compiles with the `FCL_USE_X64_SSE` option enabled, that compiles the library 
with SSE flags enabled. This creates problems in any compiler that do not targets x86 or x86_64 . 

While the problem can be easily solved by user by setting the FCL_USE_X64_SSE to OFF 
(as done for example in Debian packaging scripts, see https://salsa.debian.org/science-team/fcl/-/blob/61641fb7a12d19806c799503fc90fcd64b20c5ad/debian/rules#L10)  it is nice to provide a working 
build out of the box.
Copy link
Member

@sherm1 sherm1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:, thanks.

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@sherm1 sherm1 merged commit 83a1af6 into flexible-collision-library:master May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants