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

Remove IA32 Darwin support #2278

Merged
merged 6 commits into from Mar 7, 2019

Conversation

@mshinwell
Copy link
Contributor

commented Mar 4, 2019

This patch removes support for 32-bit Darwin (macOS, iOS, etc) targets on Intel hardware. This enables various special cases to be removed in the i386 backend that will in turn simplify a future pull request regarding symbol handling.

The current version of iOS does not support execution of 32-bit binaries any more, as far as I know. I believe the current version of macOS (Mojave) is the last one that will support 32-bit binaries.

@nojb

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

README.adoc also needs to be updated.

@nojb

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

There are a couple more mentions of macosx in configure.ac which I think can be cleaned up.

@mshinwell

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

Ah yes, I missed those as I was only looking for "darwin" there.
For some reason autogen produced changes in configure apart from the ones expected, so I manually deleted those parts of the diff.

@shindere

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

Copy link
Contributor

left a comment

Looks good to me. One suggestion and one question below.

Changes Outdated
@@ -3,6 +3,12 @@ Working version

(Changes that can break existing programs are marked with a "*")

### Code generation and optimizations:

- GPR#2278: Remove native code generation support for 32-bit Intel Darwin

This comment has been minimized.

Copy link
@xavierleroy

xavierleroy Mar 4, 2019

Contributor

"Darwin" is a code name from the early 2000's. Just call it "MacOS".

This comment has been minimized.

Copy link
@mshinwell

mshinwell Mar 5, 2019

Author Contributor

I think Darwin should be mentioned as there have been non-Mac systems using that kernel. I've updated the entry to mention macOS and iOS too.

@@ -13526,12 +13522,6 @@ case $host in #(
arch=i386; system=beos ;; #(
i[3456]86-*-cygwin) :
arch=i386; system=cygwin ;; #(
i[3456]86-*-darwin*) :

This comment has been minimized.

Copy link
@xavierleroy

xavierleroy Mar 4, 2019

Contributor

Given the if $arch64 below, it looks like it is / was possible to have a 64-bit x86 MacOS system reported as i[3456]86-*-darwin. Are we sure this is no longer the case?

This comment has been minimized.

Copy link
@mshinwell

mshinwell Mar 5, 2019

Author Contributor

I'm sure enough. I did some investigation on the web and also looked at our config.guess file. It seems to me that it is unlikely we would obtain a target triplet that didn't start x86_64- on any vaguely recent macOS system. If anyone is trying to cross compile to iOS, they will presumably have to specify the target triplet manually in any case, and as far as I understand it the x86_64- one has been canonical for some time.

@mshinwell mshinwell force-pushed the mshinwell:no_macos_32bit branch from b511d2c to ec1ef04 Mar 5, 2019
@mshinwell

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

@xavierleroy OK to merge this?

Copy link
Contributor

left a comment

Looks good to me, with one "for the record" suggestion below.

asmcomp/i386/NOTES.md Show resolved Hide resolved
@mshinwell mshinwell force-pushed the mshinwell:no_macos_32bit branch from ec1ef04 to c33aa25 Mar 7, 2019
Copy link
Contributor

left a comment

Thank you very much. Good to go, merging!

@xavierleroy xavierleroy merged commit 784c9da into ocaml:trunk Mar 7, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.