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

Fix macos triple #296

Merged
merged 1 commit into from
Apr 8, 2024
Merged

Conversation

hovsater
Copy link
Contributor

@hovsater hovsater commented Apr 6, 2024

I've been investigating this issue and it's related to the default target triple produced by Crystal during compilation. It seems like in order to fix a bug back in 2015, we stripped the minimum deployment target from the target triple in this commit. With the release of Xcode 15, a new linker was introduced and with it came these warning messages. 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.

We still need to handle the issue of normalization that's happening here, probably by not normalizing the macOS target triple at all and I'll do that in a follow up pull request.

@hovsater hovsater force-pushed the fix-macos-triple branch 2 times, most recently from 706af64 to 43148be Compare April 6, 2024 10:52
With the release of Xcode 15, a new linker was introduced and with it
came warnings from the linker. 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 also:

* https://developer.apple.com/documentation/xcode-release-notes/xcode-15-release-notes#Linking
* crystal-lang/crystal#13846
@hovsater
Copy link
Contributor Author

hovsater commented Apr 6, 2024

@straight-shoota would be possible to run a test build with these changes to see how the default target triple comes out?

@hovsater
Copy link
Contributor Author

hovsater commented Apr 7, 2024

Seems like it's working! Here's the output of the generated artefact:

$ crystal -v
Crystal 1.12.0-dev [2800d9462] (2024-04-06)                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                             LLVM: 15.0.7
Default target: aarch64-apple-macosx11.0

It subsequently also silences the warning messages as the default target triple is arm64-apple-macosx11.0 and thus, it bypasses the normalization process. The only reason Crystal report it as aarch64-apple-macosx11.0 is due to the architecture normalization done in Crystal::Codegen::Target.

@straight-shoota like you mentioned in crystal-lang/crystal#13846 (comment), we should probably discuss whether or not it makes sense to keep the normalization done in LLVM.default_target_triple. At least on macOS.

@ysbaddaden
Copy link

@hovsater as far as I recall the normalization was meant to 1. make sure we use a single flag, whatever the triple name (e.g. armhf -> arm, amd64 -> x86_64) and 2. generate a triple for choosing the correct LibC binding (src/lib_c).

Now, forcing the normalization of the actual elements was probably not a good idea. We might want to have #normalize and #normalized_architecture instead to handle the above cases, and keep the explicit triple.

@hovsater
Copy link
Contributor Author

hovsater commented Apr 7, 2024

@ysbaddaden that makes sense. We do make some normalization in Crystal::Codegen::Target which gets passed whatever the default target triple is. With this change, it seems to be working fine. The architecture is normalized for any consumers of the host target, while the actual triple is left alone. 🙂

Of course, this is only applicable on macOS currently, since the arm64 prefix bypasses the normalization done in LLVM.default_target_triple.

@straight-shoota
Copy link
Member

FTR: This change only affects the ARM build of the compiler. It's not expected to remove linker warnings on Intel macs.

@hovsater
Copy link
Contributor Author

hovsater commented Apr 8, 2024

Yeah, you're right. We do not set the target triple explicitly for x86_64, so I assume we're already using an appropriate target triple. When we follow up with the pull request addressing normalization, it should sort itself out, I believe.

@straight-shoota straight-shoota merged commit 46c295d into crystal-lang:master Apr 8, 2024
@hovsater hovsater deleted the fix-macos-triple branch April 8, 2024 18:38
hovsater added a commit to hovsater/crystal that referenced this pull request Apr 9, 2024
Starting with Xcode 15, the minimum deployment target is required in the
target triple.

See also:

* crystal-lang/distribution-scripts#296
hovsater added a commit to hovsater/crystal that referenced this pull request Apr 9, 2024
Starting with Xcode 15, the minimum deployment target is required in the
target triple.

See also:

* crystal-lang/distribution-scripts#296
straight-shoota pushed a commit to crystal-lang/crystal that referenced this pull request Jun 15, 2024
Starting with Xcode 15, the minimum deployment target is required in the target triple.

With the release of Xcode 15, a [new linker was introduced](https://developer.apple.com/documentation/xcode-release-notes/xcode-15-release-notes#Linking) and with it came warning messages like this:

    ld: warning: no platform load command found in '/Users/pascal/.cache/crystal/Users-pascal-Documents-tutorials-crystal-hello_world.cr/F-loat32.o', assuming: macOS

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 patch removes code which strips the minimum deployment target in `LLVM.default_target_triple` as an effort for standardization.

See also: crystal-lang/distribution-scripts#296
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.

4 participants