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

Remove explicit SSE2 compiler flag #760

Merged
merged 1 commit into from
Aug 26, 2016
Merged

Remove explicit SSE2 compiler flag #760

merged 1 commit into from
Aug 26, 2016

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented Aug 25, 2016

We add unconditionally the SSE2 compiler flag when using GCC. This is making non x86 arches to fail.

The bug report comes from Debian and the developers made a couple of good points about it:

  • SSE2 is enable by default if you run 64 bits on x86.
  • We can not assume that i386 supports it since i386 for Debian implies 686 (or greater) and SSE2 was introduced in pentium4.

I think that the impact of removing it is really low.


This change is Reviewable

@jslee02
Copy link
Member

jslee02 commented Aug 25, 2016

👍 This makes sense to me.

We could consider using -march=native option for gcc and clang that enables all instruction subsets supported by the local machine. This could be done in a separate pull request.

@j-rivero
Copy link
Contributor Author

We could consider using -march=native option for gcc and clang that enables all instruction subsets supported by the local machine. This could be done in a separate pull request.

If we go with this option, please consider to make it optional via flag. While most of the people use the same machine for compiling and running the applications. there are other cases (cross compilations, binary generation for packaging, etc) where the host used to compile is different than the computer running binaries.

@jslee02
Copy link
Member

jslee02 commented Aug 26, 2016

It seems the builds on OS X fail due to the recently released FLANN 1.9.1, which might have nothing to do with the change of this pull request. We could address the issue separately. Merging this PR.

@jslee02 jslee02 merged commit d59cd32 into dartsim:master Aug 26, 2016
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