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 support for 32-bit bitcode targets #11

Closed
wants to merge 1 commit into from

Conversation

davehylands
Copy link

We use a deployment target of iOS 9, which is why we append 9.0.0.

I'm happy to change this to be 11.0.0 (as it was originally). I've also modified the build script since we build for 32 and 64-bit targets and simulators, and also build for 32/64 bit android targets. That way our shared code is built using the same toolchain. If interested, I'd be happy to try and bundle up those changes (perhaps make the targets selectable using config.sh?)

@thombles
Copy link
Member

thombles commented Jun 5, 2020

Hi, sorry for the delay replying to this. The patch has actually been upstreamed to Rust and I was planning on removing it from this repo following the 1.44.0 release: rust-lang/rust#71970

However that doesn't necessarily mean that all your needs are met! We should check on that.

The upstreamed version of the code includes the 11.0.0. I tested building a project with a deployment target of iOS 10 and it seemed to be fine. Did you set it because you received some sort of error due to the 11, or just to make it match what you were deploying?

The upstreamed version also doesn't include this for 32-bit iOS, using the justification:

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 am ignorant of any situations where bitcode is actually useful for 32-bit iOS. Have I neglected something?

Having said all that, theoretically these new options should be overridable using custom target spec JSON. The question in my mind is whether adjustments are required to what has been upstreamed, or whether you have requirements that are so specific that they are not generally applicable. What do you think?

@davehylands
Copy link
Author

We're in a situation where we distribute a library which is used by our customers. Some of those customers are still targeting 32-bit bitcode (probably just to simplify deployment since they're generating 32-bit and 64-bit targets and want to do everything from a single build). I only set the 9.0 because that's our deployment target. If we're able to override using JSON, then that's great. We currently need to build our own toolchain anyways since we still need to support 32-bit targets and we also build for the simulators and android (even though we don't use bitcode under android, we want to build our android libraries using the same toolchain that we use with for iOS.

We use our own fork of this repo, mostly with modifications to the targets being built, so if this PR doesn't get merged, its not really an issue. I was mostly just sharing in case this was useful to somebody else.

@thombles
Copy link
Member

Aha I see now. I think you might get away without doing anything special here. You should be able to use -Cembed-bitcode=yes (or the older -Z option) to cause bitcode to be generated for armv7-apple-ios. I guess you've already sorted out LLVM compatibility for whatever version of Xcode you're using that still supports 32-bit targets. Presumably that will continue to work.

As far as I know the only benefit of setting this llvm cmdline is when submitting a bitcode-enabled app to the app store. That's clearly out of the question for 32-bit devices, so perhaps the cmdline-less bitcode is sufficient for your customers? They can at least have ENABLE_BITCODE=YES across the board, and that should keep Xcode happy for deploying to physical hardware etc.

If I'm wrong and you truly do need to set that field you could certainly use a custom target to override it. The aarch64 JSON has a line like this that you would want to add for the custom 32-bit target:

{
    ...
    "bitcode-llvm-cmdline": "-triple\u0000arm64-apple-ios11.0.0\u0000-emit-obj\u0000-disable-llvm-passes\u0000-target-abi\u0000darwinpcs\u0000-Os\u0000",

Alternatively, in lieu of this PR you could submit the change to rustc and add a suitable cmdline string to this struct. I doubt that would be controversial. https://github.com/rust-lang/rust/blob/master/src/librustc_target/spec/armv7_apple_ios.rs

Ultimately it doesn't make sense to merge - if this change is needed it belongs upstream now. I'll close this PR but if you have any more thoughts about the above please do comment!

@thombles thombles closed this Jun 10, 2020
@thombles
Copy link
Member

whatever version of Xcode you're using that still supports 32-bit targets

I just realised I'm mistaken about the first part - I wasn't aware that Xcode 11 still lets you work on devices as far back as iOS 8. If you can indeed still submit 32+64 bit apps then that cmdline change likely makes sense, in which case one of the latter two options is the way to go.

@davehylands
Copy link
Author

Yeah - our customer was getting build failures due to the cmdline mismatch for 32-bit apps (we already had the 64-bit cmdline fix in). We still target iOS 9 using XCode 11.3.

We still need to build a custom toolchain as long as we have to support 32-bit apple targets.

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.

2 participants