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

Disable msse2 flags on Apple Silicon #3327

Merged
merged 2 commits into from
Oct 5, 2021

Conversation

deniskoronchik
Copy link
Contributor

Issue

What issue is this PR targeting? If there is no issue that addresses the problem, please open a corresponding issue and link it here.

Tasklist

  • Add tests
  • Add #fixes with the issue number that this PR addresses
  • Update the docs with any new request parameters or changes to behavior described
  • Update the changelog
  • If you made changes to the lua files, update the taginfo too.

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

@@ -108,7 +108,7 @@ function(valhalla_module)
# These options do not affect the ABI and should therefore be used whenever possible for
# predictable numerical results
# For the x86-64 compiler, these extensions are enabled by default.
if(CMAKE_SYSTEM_PROCESSOR MATCHES "amd64|AMD64|x64|x86|x86_64")
if(CMAKE_SYSTEM_PROCESSOR MATCHES "amd64|AMD64|x64|x86|x86_64") AND NOT CMAKE_OSX_ARCHITECTURES MATCHES "arm64")
Copy link
Member

Choose a reason for hiding this comment

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

strange, that seems redundant to me. i guess on ios builds it advertizes itself as x86 through CMAKE_SYSTEM_RPOCESSOR variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CMAKE_SYSTEM_RPOCESSOR = x86_64 when we build it with arm64. I will figure out more soon, maybe some of dependencies override it

Copy link
Contributor

Choose a reason for hiding this comment

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

@kevinkreiser , it fixes an issue with new Macs on arm64.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CMAKE_OSX_ARCHITECTURES=arm64 setup -arch arm64 flag to compiler. I don't know, why CMAKE_SYSTEM_PROCESSOR=x86_64

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

oh i see its for the new mac arm architectures, thanks

@deniskoronchik deniskoronchik force-pushed the m1-support branch 2 times, most recently from 26f748a to 5f76013 Compare September 17, 2021 15:08
kevinkreiser
kevinkreiser previously approved these changes Sep 17, 2021
averkhaturau
averkhaturau previously approved these changes Sep 28, 2021
@averkhaturau averkhaturau merged commit 08fc668 into valhalla:master Oct 5, 2021
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.

3 participants