Replace `f16` and `f128` pattern matching stubs with real implementations #123088

tgross35 · 2024-03-26T10:05:58Z

This section of code depends on rustc_apfloat rather than our internal types, so this is one potential ICE that we should be able to melt now.

r? @Nadrieril

rustbot · 2024-03-26T10:06:07Z

Some changes occurred in exhaustiveness checking

cc @Nadrieril

tgross35 · 2024-03-26T10:06:48Z

Happy to add a test here if you can point me in the right direction, even though what we can actually test is probably pretty limited at this point.

@rustbot label +F-f16_and_f128

Nadrieril · 2024-03-26T10:10:54Z

👍 r=me when CI passes

tgross35 · 2024-03-26T10:12:24Z

Whoops I forgot to do the other half of that enum, thanks for the quick look

Nadrieril · 2024-03-26T10:16:09Z

Ah yeah, if you have enough implemented a small test would be nice. For example checking that a match expression with two identical -2.0..2.0 patterns lints the second one as unreachable

tgross35 · 2024-03-26T10:18:25Z

Sure, I can do that. Is there any home for pattern matching tests? Nothing in tests/ui jumps out to me

Edit: oh of course, tests/ui/match

tgross35 · 2024-03-26T10:41:37Z

Looks like I am unfortunately jumping the gun here, can't add a reasonable test until I get some more support in core such as #122470, which has its own blockers.

I'll just leave this be until then

@rustbot blocked

Nadrieril · 2024-03-26T12:21:50Z

The home for exhaustiveness tests is tests/ui/pattern/usefulness, there's already a file that tests float patterns somewhere there probably

Nadrieril · 2024-03-26T12:24:50Z

What you're testing with the tests you added is the execution of matches, i.e. match MIR lowering and codegen. The code you modify in this PR is match checking which is about exhaustiveness and the "unreachable pattern" lint

compiler/rustc_pattern_analysis/src/constructor.rs

Nadrieril · 2024-03-26T18:30:20Z

ping me again when this is unblocked so I can have a final look

tgross35 · 2024-03-26T19:03:23Z

Sounds good, I'll probably just wait for #122470 to land. Thanks for the help!

bors · 2024-03-31T13:19:06Z

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

rustbot · 2024-06-21T07:26:19Z

Some changes occurred in match checking

cc @Nadrieril

tgross35 · 2024-06-21T07:54:48Z

@Nadrieril I think this is ready for a look now, basic library support got merged so that is no longer blocking anything. There are a handful of FIXMEs still because we don't have the INFINITY constants (coming in #126608), but I don't think any of those are load bearing.

For tests I just updated everything that contains a f64 and seems relevant.

@rustbot ready

Nadrieril · 2024-06-22T17:02:41Z

tests/ui/match/match-float.rs

+fn check_f16() {
+    const F1: f16 = 0.0;
+    const F2: f16 = -0.0;
+    assert_eq!(F1, F2);


Test failed here. Assuming you ran the tests on your machine, is this platform-specific behavior? Sounds like a nightmare

That said, this is unrelated to this PR since this PR only deals with exhaustiveness checking of floats. You could comment these tests out and fix that in a different PR.

This will be failing due to #123885, which will be fixed once compiler-builtins is updated (see #125016).

Ah yeah if these are hitting any runtime tests then there will be problems on some platforms, I thought everything was going through const eval. I’ll cfg the relevant parts out when I’m back at a computer.

Nadrieril · 2024-06-22T17:03:30Z

tests/ui/half-open-range-patterns/half-open-range-pats-semantics.rs

+    // FIXME(f16_f128): enable infinity tests when constants are available
+    // assert!(yes!(f16::NEG_INFINITY, ..=f16::NEG_INFINITY));
+    // assert!(yes!(f16::NEG_INFINITY, ..=1.0f16));
+    assert!(yes!(1.5f16, ..=1.5f16));


Also a failure here

tgross35 · 2024-06-23T05:09:46Z

@Nadrieril I gated the f16 and f128 tests to only run on aarch64 Linux like I have for #126608, which is the most common platform without ABI issues or missing f128 symbols. As beetrees says, at least the x86 issues with f16 should be fixed when compiler_builtins gets bumped, the others will probably take longer.

@rustbot ready

Nadrieril · 2024-06-23T07:52:35Z

tests/ui/pattern/usefulness/floats.rs

@@ -1,4 +1,6 @@
 #![deny(unreachable_patterns)]


These will get executed as well. You can make the whole test //@ check-pass

Thanks for the catch, changed

Nadrieril · 2024-06-23T09:16:50Z

Oops I got entirely confused: that file was meant to give compile errors x) Please remove the check-pass I suggested

…ions This section of code depends on `rustc_apfloat` rather than our internal types, so this is one potential ICE that we should be able to melt now. This also fixes some missing range and match handling in `rustc_middle`.

Fixes <rust-lang#122587>

tgross35 · 2024-06-23T09:29:00Z

It fooled me too, I put it back

Nadrieril · 2024-06-23T10:05:28Z

Let's try again!

@bors r+

bors · 2024-06-23T10:05:30Z

📌 Commit 28ce7cd has been approved by Nadrieril

It is now in the queue for this repository.

bors · 2024-06-23T14:14:51Z

⌛ Testing commit 28ce7cd with merge aabbf84...

bors · 2024-06-23T16:22:49Z

☀️ Test successful - checks-actions
Approved by: Nadrieril
Pushing aabbf84 to master...

tgross35 · 2024-06-23T17:21:05Z

Hey, it worked 🎉 thanks for the review!

rust-timer · 2024-06-23T17:39:14Z

Finished benchmarking commit (aabbf84): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

Max RSS (memory usage)

Results (secondary 5.0%)

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)	5.0%	[5.0%, 5.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -2.5%)

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)	-2.5%	[-2.9%, -2.1%]	8
All ❌✅ (primary)	-	-	0

Binary size

Results (secondary -0.0%)

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)	-0.0%	[-0.0%, -0.0%]	1
All ❌✅ (primary)	-	-	0

Bootstrap: 693.516s -> 691.47s (-0.30%)
Artifact size: 326.77 MiB -> 326.79 MiB (0.01%)

rustbot assigned Nadrieril Mar 26, 2024

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

rustbot added the F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` label Mar 26, 2024

This comment has been minimized.

Sign in to view

tgross35 force-pushed the f16-f128-pattern-analysis branch from 6862b01 to 284129a Compare March 26, 2024 10:39

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2024

This comment has been minimized.

Sign in to view

Nadrieril reviewed Mar 26, 2024

View reviewed changes

compiler/rustc_pattern_analysis/src/constructor.rs Outdated Show resolved Hide resolved

tgross35 force-pushed the f16-f128-pattern-analysis branch from 284129a to 1b62868 Compare June 21, 2024 07:26

This comment has been minimized.

Sign in to view

tgross35 force-pushed the f16-f128-pattern-analysis branch from f26fabc to a125259 Compare June 21, 2024 07:52

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jun 21, 2024

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 22, 2024

Nadrieril reviewed Jun 22, 2024

View reviewed changes

Nadrieril 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 Jun 22, 2024

tgross35 force-pushed the f16-f128-pattern-analysis branch from da113aa to 9c44b60 Compare June 23, 2024 05:02

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

Nadrieril reviewed Jun 23, 2024

View reviewed changes

tgross35 force-pushed the f16-f128-pattern-analysis branch from 9c44b60 to abf8fed Compare June 23, 2024 08:22

This comment has been minimized.

Sign in to view

tgross35 added 2 commits June 23, 2024 04:28

Change a fixed crash test to a standard test

28ce7cd

Fixes <rust-lang#122587>

tgross35 force-pushed the f16-f128-pattern-analysis branch from abf8fed to 28ce7cd Compare June 23, 2024 09:28

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 Jun 23, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 23, 2024

bors merged commit aabbf84 into rust-lang:master Jun 23, 2024
7 checks passed

rustbot added this to the 1.81.0 milestone Jun 23, 2024

tgross35 deleted the f16-f128-pattern-analysis branch June 23, 2024 17:21

Replace f16 and f128 pattern matching stubs with real implementations #123088

Replace f16 and f128 pattern matching stubs with real implementations #123088

Conversation

tgross35 commented Mar 26, 2024

rustbot commented Mar 26, 2024

tgross35 commented Mar 26, 2024

This comment has been minimized.

Nadrieril commented Mar 26, 2024

tgross35 commented Mar 26, 2024

Nadrieril commented Mar 26, 2024

tgross35 commented Mar 26, 2024 • edited Loading

tgross35 commented Mar 26, 2024

This comment has been minimized.

Nadrieril commented Mar 26, 2024

Nadrieril commented Mar 26, 2024 • edited Loading

Nadrieril commented Mar 26, 2024

tgross35 commented Mar 26, 2024

bors commented Mar 31, 2024

rustbot commented Jun 21, 2024

This comment has been minimized.

tgross35 commented Jun 21, 2024 • edited Loading

This comment has been minimized.

Nadrieril Jun 22, 2024 • edited Loading

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

tgross35 commented Jun 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Nadrieril commented Jun 23, 2024

tgross35 commented Jun 23, 2024

Nadrieril commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

tgross35 commented Jun 23, 2024

rust-timer commented Jun 23, 2024

Overall result: ❌ regressions - no action needed

Replace `f16` and `f128` pattern matching stubs with real implementations #123088

Replace `f16` and `f128` pattern matching stubs with real implementations #123088

tgross35 commented Mar 26, 2024 •

edited

Loading

Nadrieril commented Mar 26, 2024 •

edited

Loading

tgross35 commented Jun 21, 2024 •

edited

Loading

Nadrieril Jun 22, 2024 •

edited

Loading