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 arm, i386 native-code backends #11904

Merged
merged 15 commits into from
Jan 22, 2023
Merged

Remove arm, i386 native-code backends #11904

merged 15 commits into from
Jan 22, 2023

Conversation

nojb
Copy link
Contributor

@nojb nojb commented Jan 17, 2023

These 32-bit backends will not be coming back. (See discussion at #9945 (comment)).

@avsm
Copy link
Member

avsm commented Jan 17, 2023

This does, incidentally, also help OpenBSD since the current i386 native code generation is incompatible with the new mimmutable(2) feature, so that was going to go bytecode only anyway.

@dbuenzli
Copy link
Contributor

Ah the joys of deleting code I envy you @nojb. That likely warrants a changes entry though :-)

One question, does 32-bit support via bytecode remain viable (the diff seems to indicate so) or do the multicore runtime system changes make that non workable as well ?

@nojb
Copy link
Contributor Author

nojb commented Jan 17, 2023

That likely warrants a changes entry though :-)

Indeed, one has been added. Thanks.

One question, does 32-bit support via bytecode remain viable (the diff seems to indicate so) or do the multicore runtime system changes make that non workable as well ?

Yes, bytecode is already working in 32-bit (except for MSVC if memory serves).

@anmonteiro
Copy link
Contributor

Just noting that there were some objections in #10540 regarding cross-compilation

@xavierleroy
Copy link
Contributor

The point raised in #10540 was that the i386 port is useful to cross-compile to arm (and other 32-bit native-code platforms). Here we're getting rid of i386 and arm at the same time, and there are no 32-bit native-code platforms left to support.

Copy link
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I only had a quick look. There's a couple of simplifications from #10540 that you could include:

  • i386-specific code in runtime/caml/stacks.h
  • removal of Proc.regs_are_volatile and its uses (good riddance!)

@nojb
Copy link
Contributor Author

nojb commented Jan 17, 2023

There's a couple of simplifications from #10540 that you could include:

  • i386-specific code in runtime/caml/stacks.h
  • removal of Proc.regs_are_volatile and its uses (good riddance!)

Fixed, thanks!

@stedolan
Copy link
Contributor

While you're at it, there's some tricky code in Cmmgen / Cmm_helpers to implement 64-bit arithmetic on 32-bit backends that can also be deleted. (Delete simplif_primitive_32bits from cmm_helpers.ml, and replace all occurrences of size_int = 4 and size_int = 8 with false and true respectively in cmmgen.ml and cmm_helpers.ml)

@nojb
Copy link
Contributor Author

nojb commented Jan 17, 2023

While you're at it, there's some tricky code in Cmmgen / Cmm_helpers to implement 64-bit arithmetic on 32-bit backends that can also be deleted. (Delete simplif_primitive_32bits from cmm_helpers.ml, and replace all occurrences of size_int = 4 and size_int = 8 with false and true respectively in cmmgen.ml and cmm_helpers.ml)

Thanks for the pointer, I pushed a commit with these simplifications.

@avsm
Copy link
Member

avsm commented Jan 17, 2023

There's also the FUNCTION_SECTIONS check for i386 in configure.ac that can be trimmed.

@nojb
Copy link
Contributor Author

nojb commented Jan 18, 2023

There's also the FUNCTION_SECTIONS check for i386 in configure.ac that can be trimmed.

Thanks, fixed.

@dra27
Copy link
Member

dra27 commented Jan 18, 2023

In Feb last year, we’d agreed in principle to keep ocamlopt word-size independent - do we have to be doing the word size simplifications at this point?

As it happens, a few of us were planning on hacking on single-domain arm32 next week (with the intention of the backend possibly existing out-of-tree)

Should the mention of ppc32 be removed from the README at this point, if we’re committing to excising all 32bit platforms (for native code)?

@avsm
Copy link
Member

avsm commented Jan 18, 2023

I think it's really important to be clear that we are not removing support for 32-bit code entirely from OCaml, and that it will continue to be supported in the bytecode backend. This is necessary as libraries will still need to check for a 32-bit wordsize, e.g. optint. And for operating system packagers, that it is supported to bundle the bytecode compilers for i386/arm32 with the expectation that they will work in a single-domain mode so that existing OCaml applications that they have already packaged (like Unison) will continue to function (albeit more slowly).

Copy link
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

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

Looks good to me. Minor suggestion below.

README.adoc Outdated
| ARM 64 bits | Linux, macOS | FreeBSD
| ARM 32 bits | Linux | FreeBSD, NetBSD, OpenBSD
| Power 64 bits | Linux |
| Power 32 bits | Linux |
Copy link
Contributor

@xavierleroy xavierleroy Jan 20, 2023

Choose a reason for hiding this comment

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

(Argh, wrong line selected, it should have been the "Power 32 bits" line.)

As pointed out by @dra27, this line should be removed, as PPC32 is not coming back either. (PPC should be back some day, but in 64-bit little-endian mode only.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed.

@xavierleroy
Copy link
Contributor

This is the kind of PR that cannot wait for too long. It seems we're blocked on @dra27's comment:

In Feb last year, we’d agreed in principle to keep ocamlopt word-size independent - do we have to be doing the > word size simplifications at this point?

I remember agreeing to keep the runtime system, the bytecode compiler and the standard library word-size independent, so that the bytecode implementation of OCaml keeps working on 32-bit platforms.

For the native-code compiler, I don't really see the point of keeping vestigial support for 32-bit platforms.

As it happens, a few of us were planning on hacking on single-domain arm32 next week (with the intention of the backend possibly existing out-of-tree)

You're on your own here. Have fun and base it on trunk before this PR is merged.

@lthls
Copy link
Contributor

lthls commented Jan 20, 2023

For the native-code compiler, I don't really see the point of keeping vestigial support for 32-bit platforms.

I should note that @chambart and @zapashcanon are working on a Wasm backend, which will be a 32-bit target. However they will likely branch quite early from the rest of the backend, with the current prototype generating Wasm code from Flambda directly, so dropping support for 32-bit targets from Cmm onwards may not be a problem for that use case. I hope that one of them will be able to confirm this.

@nojb
Copy link
Contributor Author

nojb commented Jan 22, 2023

For the native-code compiler, I don't really see the point of keeping vestigial support for 32-bit platforms.

I should note that @chambart and @zapashcanon are working on a Wasm backend, which will be a 32-bit target. However they will likely branch quite early from the rest of the backend, with the current prototype generating Wasm code from Flambda directly, so dropping support for 32-bit targets from Cmm onwards may not be a problem for that use case. I hope that one of them will be able to confirm this.

Friendly ping. @chambart @zapashcanon: can you confirm?

@zapashcanon
Copy link
Contributor

I confirm, we don't use Cmm so it's not a problem for us.

@nojb
Copy link
Contributor Author

nojb commented Jan 22, 2023

I confirm, we don't use Cmm so it's not a problem for us.

Thanks for the confirmation!

@nojb nojb merged commit eb04c8b into ocaml:trunk Jan 22, 2023
@nojb nojb deleted the remove_arm_i386 branch January 22, 2023 12:55
avsm added a commit to avsm/ocaml that referenced this pull request Mar 5, 2023
xavierleroy added a commit to xavierleroy/ocaml that referenced this pull request Oct 22, 2023
…ions

They were introduced in ocaml#2146 to support unboxed 64-bit integer
operations on 32-bit platforms, something that is no longer needed in
OCaml 5 and was removed in ocaml#11904.
xavierleroy added a commit to xavierleroy/ocaml that referenced this pull request Oct 24, 2023
…ions

They were introduced in ocaml#2146 to support unboxed 64-bit integer
operations on 32-bit platforms, something that is no longer needed in
OCaml 5 and was removed in ocaml#11904.
xavierleroy added a commit to xavierleroy/ocaml that referenced this pull request Oct 24, 2023
…ions

They were introduced in ocaml#2146 to support unboxed 64-bit integer
operations on 32-bit platforms, something that is no longer needed in
OCaml 5 and was removed in ocaml#11904.
shym pushed a commit to shym/ocaml that referenced this pull request Jan 18, 2024
shym pushed a commit to shym/ocaml that referenced this pull request Jan 19, 2024
shym pushed a commit to shym/ocaml that referenced this pull request Jan 22, 2024
shym pushed a commit to shym/ocaml that referenced this pull request Jan 30, 2024
shym pushed a commit to shym/ocaml that referenced this pull request Jan 30, 2024
shym pushed a commit to shym/ocaml that referenced this pull request Jan 31, 2024
shym pushed a commit to shym/ocaml that referenced this pull request Feb 1, 2024
dra27 added a commit to dra27/ocaml that referenced this pull request Feb 1, 2024
dra27 added a commit to dra27/ocaml that referenced this pull request Feb 1, 2024
dra27 added a commit to dra27/ocaml that referenced this pull request Feb 1, 2024
shym pushed a commit to shym/ocaml that referenced this pull request Feb 12, 2024
shym pushed a commit to shym/ocaml that referenced this pull request Feb 13, 2024
dra27 added a commit to dra27/ocaml that referenced this pull request Feb 13, 2024
shym pushed a commit to shym/ocaml that referenced this pull request Mar 11, 2024
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.

None yet

9 participants