-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Improve bitcode generation for Apple platforms #71970
Conversation
At this time Apple recommends Bitcode be included for iOS apps, and requires it for tvOS. It is unlikely that a developer would want to disable bitcode when building for these targets, yet by default it will not be generated. This presents a papercut for developers on those platforms. Introduces a new TargetOption boolean key for specific triples to indicate that bitcode should be generated, even if cargo attempts to optimise with -Cembed-bitcode=no.
The App Store performs certain sanity checks on bitcode, including that an acceptable set of command line arguments was used when compiling a given module. For Rust code to be distributed on the app store with bitcode rustc must pretend to have the same command line arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me, thanks! Just one minor thought but otherwise r=me
@@ -147,6 +148,8 @@ impl ModuleConfig { | |||
|| sess.opts.cg.linker_plugin_lto.enabled() | |||
{ | |||
EmitObj::Bitcode | |||
} else if sess.target.target.options.forces_embed_bitcode { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this get folded into need_crate_bitcode_for_rlib
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting. Right now I can't see any code path for embedding Marker
bitcode. Do we need it? IMO probably not. Would it make any sense for OptLevel
to downgrade the wishes of my target spec from Full
to Marker
? Again probably not. It's certainly surprising, and it complicates the situation for someone debugging Rust in Xcode.
So at the risk of being over-enthusiastic, I've codified this by removing the Marker
variant along with the ineffectual OptLevel
match. Do you reckon this makes sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh thanks for the keen eye! I think that may have actually been an issue from an earlier one of my PRs, but at this point I think it's safe to remove. We can always add it back in later if truly necessary.
@bors: r+ |
📌 Commit 4fea9cd has been approved by |
Rollup of 6 pull requests Successful merges: - rust-lang#71581 (Unify lints handling in rustdoc) - rust-lang#71710 (Test for zero-sized function items not ICEing) - rust-lang#71970 (Improve bitcode generation for Apple platforms) - rust-lang#71975 (Reduce `TypedArena` creations in `check_match`.) - rust-lang#72003 (allow wasm target for rustc-ap-rustc_span) - rust-lang#72017 (Work around ICEs during cross-compilation for target, ast, & attr) Failed merges: r? @ghost
Skimming PRs on TWiR, I read this as "Improve bitcoin generation for Apple platforms" 😆 |
…orse, r=davidtwco Remove legacy bitcode defaults from all Apple specs Xcode 14 [deprecated bitcode with warnings](https://developer.apple.com/documentation/xcode-release-notes/xcode-14-release-notes#Deprecations) and now [Xcode 15 has dropped it completely](https://developer.apple.com/documentation/xcode-release-notes/xcode-15-release-notes#Deprecations). `rustc` should follow what the platform tooling is doing as well since it just increases binary sizes for no gain at this point. `cc` made a [similar change last month](rust-lang/cc-rs#812). Two things show this should have minimal impact: - Apple has stopped accepting apps built with versions of Xcode (<14) that generate bitcode - The app store has been stripping bitcode off IPA releases for over 2 years now. I didn't nuke all the bitcode changes added in rust-lang#71970 since maybe another target in the future could need mandatory bitcode embedding. Staticlibs built for iOS still link correctly with XCode 15 against a test app when using a compiler built from this branch. cc `@thomcc` `@keith`
…avidtwco Remove legacy bitcode defaults from all Apple specs Xcode 14 [deprecated bitcode with warnings](https://developer.apple.com/documentation/xcode-release-notes/xcode-14-release-notes#Deprecations) and now [Xcode 15 has dropped it completely](https://developer.apple.com/documentation/xcode-release-notes/xcode-15-release-notes#Deprecations). `rustc` should follow what the platform tooling is doing as well since it just increases binary sizes for no gain at this point. `cc` made a [similar change last month](rust-lang/cc-rs#812). Two things show this should have minimal impact: - Apple has stopped accepting apps built with versions of Xcode (<14) that generate bitcode - The app store has been stripping bitcode off IPA releases for over 2 years now. I didn't nuke all the bitcode changes added in rust-lang/rust#71970 since maybe another target in the future could need mandatory bitcode embedding. Staticlibs built for iOS still link correctly with XCode 15 against a test app when using a compiler built from this branch. cc `@thomcc` `@keith`
…avidtwco Remove legacy bitcode defaults from all Apple specs Xcode 14 [deprecated bitcode with warnings](https://developer.apple.com/documentation/xcode-release-notes/xcode-14-release-notes#Deprecations) and now [Xcode 15 has dropped it completely](https://developer.apple.com/documentation/xcode-release-notes/xcode-15-release-notes#Deprecations). `rustc` should follow what the platform tooling is doing as well since it just increases binary sizes for no gain at this point. `cc` made a [similar change last month](rust-lang/cc-rs#812). Two things show this should have minimal impact: - Apple has stopped accepting apps built with versions of Xcode (<14) that generate bitcode - The app store has been stripping bitcode off IPA releases for over 2 years now. I didn't nuke all the bitcode changes added in rust-lang/rust#71970 since maybe another target in the future could need mandatory bitcode embedding. Staticlibs built for iOS still link correctly with XCode 15 against a test app when using a compiler built from this branch. cc `@thomcc` `@keith`
…avidtwco Remove legacy bitcode defaults from all Apple specs Xcode 14 [deprecated bitcode with warnings](https://developer.apple.com/documentation/xcode-release-notes/xcode-14-release-notes#Deprecations) and now [Xcode 15 has dropped it completely](https://developer.apple.com/documentation/xcode-release-notes/xcode-15-release-notes#Deprecations). `rustc` should follow what the platform tooling is doing as well since it just increases binary sizes for no gain at this point. `cc` made a [similar change last month](rust-lang/cc-rs#812). Two things show this should have minimal impact: - Apple has stopped accepting apps built with versions of Xcode (<14) that generate bitcode - The app store has been stripping bitcode off IPA releases for over 2 years now. I didn't nuke all the bitcode changes added in rust-lang/rust#71970 since maybe another target in the future could need mandatory bitcode embedding. Staticlibs built for iOS still link correctly with XCode 15 against a test app when using a compiler built from this branch. cc `@thomcc` `@keith`
Some improvements for iOS bitcode support suggested by Alex over at getditto/rust-bitcode#9. r? @alexcrichton
This improves Rust's bitcode generation so that provided you have a compatible LLVM version, Rust targeting iOS should work out of the box when compiled into bitcode-enabled apps, and when submitted to the App Store. I've tested these changes using Xcode 11.4.1 and Apple's vendored LLVM, tag
swift-5.2.3-RELEASE
.aarch64-apple-ios
andaarch64-apple-tvos
targets to always emit full bitcode sections, even when cargo is trying to optimise by invokingrustc
with-Cembed-bitcode=no
. Since Apple recommends bitcode on iOS and requires it on tvOS it is likely that this is what developers intend. Currently you need to override the codegen options withRUSTFLAGS
, which is far from obvious.TargetOptions
for iOS, copied directly from the a clang equivalent section.In the context of Apple platforms, the predominant purpose of bitcode is App Store submissions, so simulator and 32-bit targets are not relevant. I'm hoping that the cmdline strings will not be a maintenance burden to keep up-to-date. If the event of any future incompatibilities, hopefully a custom target config would offer enough flexibility to work around it. It's impossible to say for sure.
Due to unrelated build errors I haven't been able to build and test a full tvOS toolchain. I've stopped short of providing a similar
bitcode_llvm_cmdline
until I can actually test it.