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

DM-32487: Adjust flags and tests to support osx-arm64 #203

Merged
merged 2 commits into from Nov 8, 2021

Conversation

erykoff
Copy link
Contributor

@erykoff erykoff commented Nov 8, 2021

Note that the astropy vs Rubin proper motion comparison test needed to be relaxed because the astropy value (used as a comparison) is varies depending on architecture.


uname = os.uname()
if uname.sysname == 'Darwin' and uname.machine == 'arm64':
flags.append('-faligned-allocation')
Copy link
Member

Choose a reason for hiding this comment

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

Is there a down side to using that on intel macOS as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it ain't broke...
But I don't know.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it's only available for C++17 compilers.

-faligned-allocation    Enable C++17 aligned allocation functions

We are meant to be using C++17 these days but this is presumably fall out from us insisting on using an old macOS deployment target on Intel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, a little more research (and as noted in the ticket description) I think that the deployment target that we have as a default on x86 will make this impossible to have as the universal default.

Copy link
Contributor

Choose a reason for hiding this comment

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

Enabling C++17 compilation doesn't necessarily enable library support for it. MacOSx way to be backwards
compatible with old deployment targets.

@erykoff erykoff merged commit 247e844 into master Nov 8, 2021
@erykoff erykoff deleted the tickets/DM-32487 branch November 8, 2021 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants