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

Add platform docs for FreeBSD. #129220

Merged
merged 1 commit into from
Dec 23, 2024
Merged

Conversation

asomers
Copy link
Contributor

@asomers asomers commented Aug 17, 2024

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Aug 17, 2024

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 17, 2024
@asomers
Copy link
Contributor Author

asomers commented Aug 17, 2024

cc @tgross35 . Also, perhaps @MikaelUrankar would like to sign up as a second target maintainer?

@rust-log-analyzer

This comment has been minimized.

@ehuss
Copy link
Contributor

ehuss commented Aug 18, 2024

r? ehuss

Looks like we have two PRs updating the same thing over at #128546.

Can we combine them? Or I can merge #128546 now, until this is ready?

@rustbot rustbot assigned ehuss and unassigned JohnTitor Aug 18, 2024
Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

In the platform support docs, please note that FreeBSD's ports patches i686-unknown-freebsd in a way that introduces incorrect compilations, thus people must use the upstream toolchain instead of a FreeBSD-built toolchain for building for i686-unknown-freebsd without introducing incorrect compilations.

Unless this changes, of course.

@beetrees
Copy link
Contributor

Ideally (since FreeBSD wants to support pre-SSE2 CPUs), instead of FreeBSD ports patching the i686-unknown-freebsd package to be subtly different than upstream, a separate i586-unknown-freebsd target should be introduced (similar to the existing i586-unknown-linux-gnu target for Linux). That way upstream and FreeBSD ports targets can have the same behaviour, and users can pick between "I want to support really old hardware" (i586) and "I want to be able to use floats" (i686), same as they can on other operating systems.

@beetrees
Copy link
Contributor

beetrees commented Aug 18, 2024

Also, as far as I can tell, the powerpc64-unknown-freebsd target in upstream Rust supports FreeBSD 12 only as FreeBSD 13+ switched to the ELFv2 ABI, which the FreeBSD port patches the Rust compiler to use. Given that FreeBSD 12 is EOL, the solution seems to be to upstream a cx.target_spec().os == "freebsd" version of the patch (the FreeBSD ports patch currently changes all big-endian PowerPC64 targets to use ELFv2, not just FreeBSD), and just state that powerpc64-unknown-freebsd only supports FreeBSD 13+. The target description also needs to be updated: currently it states "PPC64 FreeBSD (ELFv1 and ELFv2)" which is incorrect as the compiler currently only supports the ELFv1 ABI for the target (and after the patch is upstreamed will only support the ELFv2 ABI for the target).

@MikaelUrankar
Copy link
Contributor

cc @tgross35 . Also, perhaps @MikaelUrankar would like to sign up as a second target maintainer?

Yes, you can put me as a 2nd target maintainer.

@asomers
Copy link
Contributor Author

asomers commented Aug 20, 2024

@workingjubilee @beetrees the decision to build lang/rust without SSE support was made in 2017, before any of the linked LLVM issues. I'll start a discussion about revisiting it. I'll inquire about the ELFv1/ELFv2 issue too.

@rust-log-analyzer

This comment has been minimized.

@workingjubilee
Copy link
Member

Ideally (since FreeBSD wants to support pre-SSE2 CPUs)

I am not entirely sure they do? I have been informed contrariwise, i.e. that FreeBSD is only interested in Pentium 4 or later equivalents. Naturally, that might have been decided many years after the 2017 decision!

Of course, if two targets settles the matter for FreeBSD, that is fine by me, and if a documentation change is preferred, that is also fine by me. I do not mean to be obstructive, I merely want the matter to be clear for everyone involved.

@beetrees
Copy link
Contributor

I was inferring "since FreeBSD wants to support pre-SSE2 CPUs" from the FreeBSD i386 platform page, which states "FreeBSD/i386 should support any CPU compatible with the Intel Pentium Pro / Pentium II (i686) or better" (it's quite possible that the platform page is out of date though).

It's worth noting that the FreeBSD 14.1 release notes state that 32-bit x86 platform support will be dropped from FreeBSD 15 onwards, while support for running 32-bit binaries on 64-bit x86 will remain for at least FreeBSD 15 and 16. Given that all 64-bit x86 CPUs support SSE2, there will definitely be no need for non-SSE2 CPU support after FreeBSD 14 reaches EOL (currently expected to be in November 2028).

@beetrees
Copy link
Contributor

#120869 appears relevant to this PR as it bumps the minimum supported FreeBSD version.

@bors
Copy link
Contributor

bors commented Oct 21, 2024

☔ The latest upstream changes (presumably #120869) made this pull request unmergeable. Please resolve the merge conflicts.

@ehuss
Copy link
Contributor

ehuss commented Oct 21, 2024

Just FYI, you'll need to add the link to SUMMARY.md. Also, I'd suggest fixing the target definitions in
compiler/rustc_target/src/spec/targets if those are indeed incorrect. See #128546 for examples of these changes.

@workingjubilee
Copy link
Member

@ehuss I would rather not deliberately introduce miscompilations into existing tier 2 targets that currently have none, whether or not ports has decided to make their target unsound.

@ehuss
Copy link
Contributor

ehuss commented Oct 21, 2024

@workingjubilee Sorry, I'm not following. Are you asking that the change requested above be resolved? I'm not the author, so I wouldn't be the one making that change. As the reviewer, I won't merge until it is resolved.

Or maybe you intended to tag one of the other commenters here? Sorry, I'm confused. 😕

@workingjubilee
Copy link
Member

workingjubilee commented Oct 21, 2024

@ehuss I am saying that the resolution can include ports fixing its target definition to not make Rust code unsound, i.e. by ceasing to apply their patch, rather than it being "fixed" here by making the target unsound for our toolchain as well.

Unless that is what you meant, in which case it was just unclear to me, apologies.

@asomers
Copy link
Contributor Author

asomers commented Oct 21, 2024

A change is underway to reenable SSE on i386 by default: https://reviews.freebsd.org/D47227

@ehuss
Copy link
Contributor

ehuss commented Oct 23, 2024

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2024
@Dylan-DPC
Copy link
Member

@asomers any updates on this? thanks

@asomers
Copy link
Contributor Author

asomers commented Nov 8, 2024

@asomers any updates on this? thanks

I expect to merge that change, reenabling SSE2 on i386, by EoD on Saturday.

@tgross35
Copy link
Contributor

tgross35 commented Dec 9, 2024

Is the SSE note accurate since the push 3 weeks ago?

If not, I think it would be okay to drop that section for now and merge the rest of the docs so we at least have something.

@asomers
Copy link
Contributor Author

asomers commented Dec 9, 2024

Is the SSE note accurate since the push 3 weeks ago?

If not, I think it would be okay to drop that section for now and merge the rest of the docs so we at least have something.

Yes, the SSE note is accurate. I committed that change on 9-Nov. https://reviews.freebsd.org/R11:b039f2e46b15c1ff5cbaf41e642993d982294da3

@asomers asomers marked this pull request as ready for review December 9, 2024 01:10
@rustbot
Copy link
Collaborator

rustbot commented Dec 9, 2024

Some changes occurred in src/doc/rustc/src/platform-support

cc @Noratrieb

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

This needs an entry in SUMMARY.md and a squash, but with freebsd/freebsd-ports@b039f2e I don't think there is anything that this would be waiting on.

@bors
Copy link
Contributor

bors commented Dec 14, 2024

☔ The latest upstream changes (presumably #134296) made this pull request unmergeable. Please resolve the merge conflicts.

@tgross35
Copy link
Contributor

@asomers could you rebase and squash? I'll merge this, if any small changes are needed they can come as a followup.

@asomers
Copy link
Contributor Author

asomers commented Dec 22, 2024

@asomers could you rebase and squash? I'll merge this, if any small changes are needed they can come as a followup.

done

@tgross35
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 22, 2024

📌 Commit 12b54b1 has been approved by tgross35

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 22, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2024
Rollup of 4 pull requests

Successful merges:

 - rust-lang#129220 (Add platform docs for FreeBSD.)
 - rust-lang#134659 (test-infra: improve compiletest and run-make-support symlink handling)
 - rust-lang#134668 (Make sure we don't lose default struct value when formatting struct)
 - rust-lang#134672 (Revert stabilization of the `#[coverage(..)]` attribute)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3acf9c9 into rust-lang:master Dec 23, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2024
Rollup merge of rust-lang#129220 - asomers:target-maintainer, r=tgross35

Add platform docs for FreeBSD.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.