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

Do not strip the macOS target triple #14466

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hovsater
Copy link
Contributor

@hovsater hovsater commented Apr 9, 2024

With the release of Xcode 15, a new linker was introduced and with it came warnings messages as seen in #13846. It appears that the new linker is stricter in which target triples it considers valid. Specifically, it looks like the minimum deployment target is required. E.g., aarch64-apple-darwin23.3.0 is valid, while aarch64-apple-darwin is not. See #13846 (comment) for details.

This PR is a follow-up of crystal-lang/distribution-scripts#296. While we have corrected the target triple for macOS arm64 builds, x86_64 builds would still emit warnings due to us stripping the minimum deployment target in LLVM.default_target_triple.

Fixes #13846
Fixes #14052

@hovsater
Copy link
Contributor Author

hovsater commented Apr 9, 2024

I only address macOS here, but a discussion is worth having about whether we want to perform any stripping of the target triple in LLVM.default_target_triple. Isn't the normalization done in Crystal::Codegen::Target enough?

Starting with Xcode 15, the minimum deployment target is required in the
target triple.

See also:

* crystal-lang/distribution-scripts#296
@hovsater
Copy link
Contributor Author

Could this be merged or is there something blocking it?

@@ -91,12 +91,6 @@ module LLVM
def self.default_target_triple : String
chars = LibLLVM.get_default_target_triple
case triple = string_and_dispose(chars)
when .starts_with?("x86_64-apple-macosx"), .starts_with?("x86_64-apple-darwin")
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this still needed to remove the deployment version?

Copy link
Contributor Author

@hovsater hovsater May 3, 2024

Choose a reason for hiding this comment

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

I deliberately remove this because we want to keep the minimum deployment target. Since not specifying it, causes linker warnings as of Xcode 15. See #13846 (comment) for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

arm64 versus aarch64 in LLVM.default_target_triple Linker warnings in Mac OS Ventura
4 participants