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

Added build configuration for Apple Silicon based Macs. #1055

Closed
wants to merge 1 commit into from

Conversation

chadseld
Copy link

Added the required build configuration changes to support compilation on Apple Silicon Macs.

Tested on a Developer Transition Kit Mac.

cargo build
cargo test

For now, the flavor portion of the ASM_TARGETS triple is left as ios64. With some additional changes to arm-xlate.pl, we could insert a custom string here (e.g. macArm64).

@repi
Copy link

repi commented Oct 17, 2020

Linking together with #1063

@Darkspirit
Copy link

alexcrichton/openssl-src-rs#77 added aarch64-apple-darwin to openssl CI.

not(target_os = "ios")
not(
any(target_os = "ios",
target_os = "macos"))
Copy link
Owner

Choose a reason for hiding this comment

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

AFAICT, this would have disabled CPU feature detection on x86-64 Macs, so I didn't take this change.

Also, when I was trying to understand your PR, I noticed the bug that PR #1100 fixes. (That bug is just a performance bug, not a correctness bug, but it is a pretty severe performance bug.)

@@ -235,6 +235,7 @@ const ASM_TARGETS: &[(&str, Option<&str>, Option<&str>)] = &[
("x86_64", Some(WINDOWS), Some("nasm")),
("x86_64", None, Some("elf")),
("aarch64", Some("ios"), Some("ios64")),
("aarch64", Some("macos"), Some("ios64")),
Copy link
Owner

Choose a reason for hiding this comment

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

I agree that this is the right thing to do; this is what OpenSSL upstream does and this is what PR #1101 now does.

@@ -173,21 +175,22 @@ pub(crate) mod arm {
#[cfg_attr(
any(
target_os = "ios",
target_os = "macos",
Copy link
Owner

Choose a reason for hiding this comment

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

A lot of my recent changes to cpu.rs were to clean up this repetitive logic that existed just to avoid dead code warnings.

@briansmith
Copy link
Owner

Thanks for this PR. I used this as input for making PR #1100 and PR #1101. It was also motivation for cleaning up cpu.rs. I hope to complete more cleanup after the release.

@briansmith briansmith closed this Nov 18, 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.

4 participants