Skip to content

Update compiler-builtins and enable f128 tests on all non-buggy platforms - #132434

Merged
bors merged 2 commits into
rust-lang:masterfrom
tgross35:f128-tests
Nov 4, 2024
Merged

bors merged 2 commits into
rust-lang:masterfrom
tgross35:f128-tests

Conversation

@tgross35

@tgross35 tgross35 commented Nov 1, 2024 •

Copy link
Copy Markdown
Member

Update compiler_builtins to 0.1.138 and pin it. This updates to a new version of builtins that includes 1, which was
the last blocker to us enabling f128 tests on all platforms.

With that, we now provide symbols necessary to work with f128 everywhere. This means that we are no longer restricted to systems that provide f128 symbols themselves, and can enable tests by default.

There are still a handful of platforms that need to remain disabled because of bugs and some that had to get updated.

Math support is still off by default since those symbols are not yet available.

try-job: test-various
try-job: i686-gnu-nopt

@rustbot

rustbot commented Nov 1, 2024

Copy link
Copy Markdown
Collaborator

r? @workingjubilee

rustbot has assigned @workingjubilee.
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 A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 1, 2024
@rustbot

rustbot commented Nov 1, 2024

Copy link
Copy Markdown
Collaborator

These commits modify the library/Cargo.lock file. Unintentional changes to library/Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

@tgross35

tgross35 commented Nov 1, 2024

Copy link
Copy Markdown
Member Author

This includes #132433 which includes #132206, so the chain is a bit deep (I'm keeping updates separate from rust-lang/rust changes so we aren't blocked from updating further if something goes wrong here). But that doesn't mean we can't start some tests.

@bors try

@tgross35

tgross35 commented Nov 1, 2024

Copy link
Copy Markdown
Member Author

Those are quite a few try jobs but it still doesn't cover everything that will be newly enabled here.

@bors rollup=never

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 1, 2024
Enable f128 tests on all non-buggy platforms 🎉

With the `compiler-builtins` update to 0.1.137 [1], we now provide symbols necessary to work with `f128` everywhere. This means that we are no longer restricted to 64-bit linux, and can enable tests by default.

There are still a handful of platforms that need to remain disabled because of bugs.

Math support is still off by default since those symbols are not yet available.

[1]: rust-lang#132433

try-job: arm-android
try-job: armhf-gnu
try-job: i686-gnu
try-job: x86_64-apple-1
try-job: i686-mingw
try-job: x86_64-msvc-ext
@bors

bors commented Nov 1, 2024

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 15075bf with merge ec0220a...

@tgross35

tgross35 commented Nov 1, 2024

Copy link
Copy Markdown
Member Author

Also cc @beetrees

@bors

bors commented Nov 1, 2024

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-actions
Build commit: ec0220a (ec0220aadb3398fa8aa480a8d4a0f8e3e6d48e20)

@workingjubilee

Copy link
Copy Markdown
Member

This has a merge conflict, it seems.

@workingjubilee

Copy link
Copy Markdown
Member

I have rebased it.

@beetrees

beetrees commented Nov 3, 2024

Copy link
Copy Markdown
Contributor

I think the f128 tests need to be disabled on 32-bit x86 due to llvm/llvm-project#77401 (although tests might pass anyway if the symbols from the LLVM-compiled compiler-builtins get picked by the linker instead of those from a GCC-compiled system library).

Also mips64/mips64r6 currently doesn't have f128 builtins built for it due to llvm/llvm-project#96432.

@workingjubilee

workingjubilee commented Nov 3, 2024 •

Copy link
Copy Markdown
Member

I do not believe we run the relevant tests on mips64?

@workingjubilee

Copy link
Copy Markdown
Member

I decided to change up the suite of tests slightly and rerun this. In particular, I noticed we didn't run the infamous test-various.

@bors try

@bors

bors commented Nov 3, 2024

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 95998ee with merge ad17ccb...

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 3, 2024
Enable f128 tests on all non-buggy platforms 🎉

With the `compiler-builtins` update to 0.1.137 [1], we now provide symbols necessary to work with `f128` everywhere. This means that we are no longer restricted to 64-bit linux, and can enable tests by default.

There are still a handful of platforms that need to remain disabled because of bugs.

Math support is still off by default since those symbols are not yet available.

[1]: rust-lang#132433

try-job: armhf-gnu
try-job: i686-gnu
try-job: i686-gnu-nopt
try-job: test-various
try-job: dist-mips-linux
try-job: dist-mips64el-linux
@rust-log-analyzer

This comment has been minimized.

@beetrees

beetrees commented Nov 3, 2024

Copy link
Copy Markdown
Contributor

I do not believe we run the relevant tests on mips64?

All MIPS targets are tier 3, so no builds or tests of any kind are run by rust-lang/rust. The benefit of adding it to the match statement is just for those running tests locally/Linux distros with a MIPS64 port etc. 32-bit sparc is also tier 3 only and is already on the list.