Enable `f16` tests on non-GNU Windows #130959

tgross35 · 2024-09-27T23:00:45Z

There is a MinGW ABI bug that prevents f16 and f128 from being usable on windows-gnu targets. This does not affect MSVC; however, we have f16 and f128 tests disabled on all Windows targets.

Update the gating to only affect windows-gnu, which means f16 tests will be enabled. There is no effect for f128 since the default fallback is false.

try-job: x86_64-msvc
try-job: i686-msvc

rustbot · 2024-09-27T23:00:52Z

r? @joboet

rustbot has assigned @joboet.
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

tgross35 · 2024-09-27T23:03:37Z

#129385 should ideally merge first so this is effectively blocked. But we should at least be able to test the change.

@bors try

Cc @beetrees to double check this

…nu, r=<try> Enable `f16` tests on non-GNU Windows There is a MinGW ABI bug that prevents `f16` and `f128` from being usable on `windows-gnu` targets. This does not affect MSVC; however, we have `f16` and `f128` tests disabled on all Windows targets. Update the gating to only affect `windows-gnu`, which means `f16` tests will be enabled. There is no effect for `f128` since the default fallback is `false`. try-job: x86_64-msvc try-job: i686-msvc

bors · 2024-09-27T23:04:48Z

⌛ Trying commit a8902f0 with merge 2db5115...

bors · 2024-09-27T23:12:12Z

💔 Test failed - checks-actions

tgross35 · 2024-09-27T23:28:33Z

@bors try

bors · 2024-09-27T23:30:47Z

⌛ Trying commit 83d56df with merge ee9a311...

…nu, r=<try> Enable `f16` tests on non-GNU Windows There is a MinGW ABI bug that prevents `f16` and `f128` from being usable on `windows-gnu` targets. This does not affect MSVC; however, we have `f16` and `f128` tests disabled on all Windows targets. Update the gating to only affect `windows-gnu`, which means `f16` tests will be enabled. There is no effect for `f128` since the default fallback is `false`. try-job: x86_64-msvc try-job: i686-msvc

bors · 2024-09-28T01:15:51Z

💔 Test failed - checks-actions

workingjubilee · 2024-09-28T07:12:02Z

float tests + Windows, I can think of few more exciting combinations. already getting mysterious failures...

@bors rollup=iffy

mati865 · 2024-09-28T08:44:17Z

library/std/build.rs

@@ -104,7 +104,7 @@ fn main() {
        // Unsupported <https://github.com/llvm/llvm-project/issues/94434>
        ("arm64ec", _) => false,
        // MinGW ABI bugs <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115054>
-        ("x86_64", "windows") => false,
+        ("x86_64", "windows") if target_env == "gnu" => false,


Shouldn't it also check for target_abi == "" or target_abi != "llvm" to enable it for gnullvm targets since it works fine with Clang?

Do gnullvm targets avoid linking MinGW/GCC libraries at all? I'm honestly not positive how those works.

If you are suggesting updating the gate to if target_env == "gnu" && target_abi == "" => false I can do that after figuring out what is currently failing.

gnullvm targets are based on mingw-w64 just like regular gnu targets but built using the Clang / LLVM toolchain. In https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115054 you've said Clang is using proper ABI, so this feature should be fine. Doesn't have to be done in this PR though.
The same for f128.

Clang uses the correct ABI, but the problem is it can link symbols from libgcc which means things wind up broken. Maybe this isn't the case for gnullvm for some reason?

In any case I'll plan to leave this PR as-is for now unless you are able to check that things work on gnullvm (or I'll try to do that at some point after this merges).

gnullvm targets don't use anything from GCC/GNU, so this is not a problem. FWIW libgcc is replaced by compiler-rt and libgcc_{s,eh} is replaced by libunwind.

I'm trying to tests that, but I have troubles cross-compiling full toolchain because workspace resolver = "2" exposed Cargo bug:

Building tool rustdoc (stage1 -> stage2, x86_64-pc-windows-gnullvm) thread 'main' panicked at src/cargo\core\resolver\features.rs:323:14: activated_features for invalid package: features did not find PackageId { name: "windows_x86_64_gnullvm", version: "0.52.6", source: "registry `crates-io`" } HostDep note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

That might get a fix soon: rust-lang/cargo#14593
Otherwise I'll work around it another way, so I'll take care of that change then.

Oh that's really interesting. compiler-rt may have been part of the problem too since GCC was being used to build it - but I don't really remember exactly what I was testing, and sounds like that might not be a problem anyway.

Anyway, totally your discretion since you're probably the main one testing this target :) feel free to r? me if you wind up putting up a followup PR.

LLVM mingw-w64 toolchains are purely LLVM + mingw-w64 based, no GNU tools like GCC or Binutils are used at any point.

EDIT: feel free to resolve this discussion, my button has disappeared.

tgross35 · 2024-09-28T13:26:40Z

I have no clue what the error was and raw logs don't work - did the runner just die?

@bors try

bors · 2024-09-28T13:27:49Z

⌛ Trying commit 83d56df with merge fb032d2...

…nu, r=<try> Enable `f16` tests on non-GNU Windows There is a MinGW ABI bug that prevents `f16` and `f128` from being usable on `windows-gnu` targets. This does not affect MSVC; however, we have `f16` and `f128` tests disabled on all Windows targets. Update the gating to only affect `windows-gnu`, which means `f16` tests will be enabled. There is no effect for `f128` since the default fallback is `false`. try-job: x86_64-msvc try-job: i686-msvc

bors · 2024-09-28T15:26:01Z

☀️ Try build successful - checks-actions
Build commit: fb032d2 (fb032d2d45aca90636bdb78b89b4e4182aac62c4)

tgross35 · 2024-09-28T15:30:47Z

Woo. Still will need a rebase after #129385 so I can drop the second commit here, but this seems to have done what it's expected to.

@rustbot review

tgross35 · 2024-09-28T23:24:15Z

#129385 merged so I was able to drop the second commit.

@rustbot label -S-blocked

bors · 2024-10-01T02:35:57Z

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

There is a MinGW ABI bug that prevents `f16` and `f128` from being usable on `windows-gnu` targets. This does not affect MSVC; however, we have `f16` and `f128` tests disabled on all Windows targets. Update the gating to only affect `windows-gnu`, which means `f16` tests will be enabled. There is no effect for `f128` since the default fallback is `false`.

joboet · 2024-10-01T10:29:11Z

That appears to work, let's merge it.
@bors r+

bors · 2024-10-01T10:29:14Z

📌 Commit 6b09a93 has been approved by joboet

It is now in the queue for this repository.

bors · 2024-10-01T10:48:32Z

⌛ Testing commit 6b09a93 with merge c4f7176...

bors · 2024-10-01T13:34:09Z

☀️ Test successful - checks-actions
Approved by: joboet
Pushing c4f7176 to master...

rust-timer · 2024-10-01T14:54:18Z

Finished benchmarking commit (c4f7176): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 1.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.4%	[1.4%, 1.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.4%	[1.4%, 1.4%]	1

Cycles

Results (secondary -5.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.1%	[-7.2%, -4.2%]	4
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 771.18s -> 771.847s (0.09%)
Artifact size: 341.46 MiB -> 341.40 MiB (-0.02%)

enable f16 and f128 on windows-gnullvm targets Continuation of rust-lang#130959

Rollup merge of rust-lang#131308 - mati865:gnullvm-f16-f128, r=tgross35 enable f16 and f128 on windows-gnullvm targets Continuation of rust-lang#130959

rustbot assigned joboet Sep 27, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 27, 2024

This comment has been minimized.

Sign in to view

tgross35 mentioned this pull request Sep 27, 2024

Enable f16 tests on x86 Apple platforms #130961

Merged

tgross35 added the F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` label Sep 27, 2024

This comment has been minimized.

Sign in to view

bors 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 Sep 27, 2024

tgross35 force-pushed the f16-f128-only-disable-win-gnu branch from a8902f0 to 83d56df Compare September 27, 2024 23:28

workingjubilee added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Sep 28, 2024

This comment has been minimized.

Sign in to view

mati865 reviewed Sep 28, 2024

View reviewed changes

rustbot removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 28, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 28, 2024

tgross35 added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Sep 28, 2024

This comment was marked as outdated.

Sign in to view

tgross35 force-pushed the f16-f128-only-disable-win-gnu branch from 83d56df to 58559fc Compare September 28, 2024 23:23

rustbot removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Sep 28, 2024

tgross35 force-pushed the f16-f128-only-disable-win-gnu branch from 58559fc to 6b09a93 Compare October 1, 2024 02:42

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 1, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 1, 2024

bors merged commit c4f7176 into rust-lang:master Oct 1, 2024
7 checks passed

rustbot added this to the 1.83.0 milestone Oct 1, 2024

tgross35 deleted the f16-f128-only-disable-win-gnu branch October 1, 2024 14:19

mati865 mentioned this pull request Oct 5, 2024

enable f16 and f128 on windows-gnullvm targets #131308

Merged

Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 7, 2024

Rollup merge of rust-lang#131308 - mati865:gnullvm-f16-f128, r=tgross35

5c1c49a

enable f16 and f128 on windows-gnullvm targets Continuation of rust-lang#130959

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 7, 2024

Unrolled build for rust-lang#131308

a7a0929

Rollup merge of rust-lang#131308 - mati865:gnullvm-f16-f128, r=tgross35 enable f16 and f128 on windows-gnullvm targets Continuation of rust-lang#130959

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable `f16` tests on non-GNU Windows #130959

Enable `f16` tests on non-GNU Windows #130959

tgross35 commented Sep 27, 2024 •

edited

Loading

rustbot commented Sep 27, 2024

tgross35 commented Sep 27, 2024 •

edited

Loading

bors commented Sep 27, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Sep 27, 2024

tgross35 commented Sep 27, 2024

bors commented Sep 27, 2024

This comment has been minimized.

bors commented Sep 28, 2024

workingjubilee commented Sep 28, 2024

mati865 Sep 28, 2024

tgross35 Sep 28, 2024

mati865 Sep 28, 2024

tgross35 Sep 28, 2024

mati865 Sep 28, 2024

tgross35 Sep 28, 2024

mati865 Sep 28, 2024 •

edited

Loading

tgross35 commented Sep 28, 2024

bors commented Sep 28, 2024

bors commented Sep 28, 2024

tgross35 commented Sep 28, 2024

This comment was marked as outdated.

tgross35 commented Sep 28, 2024

bors commented Oct 1, 2024

joboet commented Oct 1, 2024

bors commented Oct 1, 2024

bors commented Oct 1, 2024

bors commented Oct 1, 2024

rust-timer commented Oct 1, 2024

Enable f16 tests on non-GNU Windows #130959

Enable f16 tests on non-GNU Windows #130959

Conversation

tgross35 commented Sep 27, 2024 • edited Loading

rustbot commented Sep 27, 2024

tgross35 commented Sep 27, 2024 • edited Loading

bors commented Sep 27, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Sep 27, 2024

tgross35 commented Sep 27, 2024

bors commented Sep 27, 2024

This comment has been minimized.

bors commented Sep 28, 2024

workingjubilee commented Sep 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mati865 Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

tgross35 commented Sep 28, 2024

bors commented Sep 28, 2024

bors commented Sep 28, 2024

tgross35 commented Sep 28, 2024

This comment was marked as outdated.

tgross35 commented Sep 28, 2024

bors commented Oct 1, 2024

joboet commented Oct 1, 2024

bors commented Oct 1, 2024

bors commented Oct 1, 2024

bors commented Oct 1, 2024

rust-timer commented Oct 1, 2024

Overall result: no relevant changes - no action needed

Enable `f16` tests on non-GNU Windows #130959

Enable `f16` tests on non-GNU Windows #130959

tgross35 commented Sep 27, 2024 •

edited

Loading

tgross35 commented Sep 27, 2024 •

edited

Loading

mati865 Sep 28, 2024 •

edited

Loading