Prepare NonNull for pattern types by oli-obk · Pull Request #152702 · rust-lang/rust

oli-obk · 2026-02-16T10:21:47Z

Pull out the changes that affect some tests, but do not require pattern types.

See #136006 (comment) for what triggered this PR

oli-obk · 2026-02-16T10:22:00Z

Prepare NonNull for pattern types

rust-bors · 2026-02-16T12:32:40Z

☀️ Try build successful (CI)
Build commit: ea5f33e (ea5f33ec13511047f4f28548657d6f308b541315, parent: fef627b1ebdc7369ddf8a4031a5d25733ac1fb34)

rust-timer · 2026-02-16T13:12:45Z

Finished benchmarking commit (ea5f33e): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	1.3%	[1.3%, 1.3%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-0.5%	[-0.5%, -0.5%]	2
All ❌✅ (primary)	0.4%	[-0.5%, 1.3%]	2

Max RSS (memory usage)

Results (primary -0.7%, secondary 8.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	4.4%	[4.4%, 4.4%]	1
Regressions ❌ (secondary)	8.5%	[8.5%, 8.5%]	1
Improvements ✅ (primary)	-3.3%	[-3.8%, -2.7%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.7%	[-3.8%, 4.4%]	3

Cycles

Results (secondary 0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Binary size

Results (primary 0.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Bootstrap: 481.396s -> 480.639s (-0.16%)
Artifact size: 397.80 MiB -> 395.76 MiB (-0.51%)

scottmcm

This basically looks good, but did leave me with two thoughts.

I have no idea how important the !align metadata is. Might be ok to just file a follow-up bug to bring it back? Dunno how hard it would be to restore.

View changes since this review

scottmcm · 2026-02-17T01:20:27Z

library/core/src/ptr/non_null.rs

+        let pointer: *const T = crate::ptr::without_provenance(addr.get());
        // SAFETY: we know `addr` is non-zero.
-        unsafe { NonNull { pointer } }
+        unsafe { NonNull { pointer: transmute(pointer) } }


up to you: if this is going to transmute anyway, maybe just transmute directly to NonNull?

Going through the usize then the pointer then the aggregate doesn't seem particularly meaningful; maybe just go straight NonZero→NonNull if there needs to be a transmute here regardless?

I also tried going from *const T to NonNull directly without going through the field constructor. It causes slightly worse MIR, because we don't really handle things like cast well in that case (NonZero transmute to raw pointer, cast, transmute back to NonZero).

scottmcm · 2026-02-17T01:26:31Z

tests/codegen-llvm/loads.rs

 #[no_mangle]
 pub fn load_box<'a>(x: Box<Box<i32>>) -> Box<i32> {
-    // CHECK: load ptr, ptr %{{.*}}, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_4_META]], !noundef !{{[0-9]+}}
+    // CHECK: load ptr, ptr %{{.*}}, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !noundef !{{[0-9]+}}


Hmm, if this is showing up here with just the nonnull change it looks like whatever emits this !align needs an update to see through pattern types, maybe?

This PR doesn't use pattern types yet 😅 so the issue is likely that we lose the information somewhere in the transmute. Lemme try your other suggestion, which maybe LLVM can preserve things if things are simpler

Oh, doh, of course it doesn't use pattern types 🤦

Note that this is a -C no-prepopulate-passes test, though, so it's only testing things that we in cg_llvm actually emit for the loads. So this implies that it's loading as a pointer instead of as a Box, somehow, I guess?

(I was going to say "well if it's just llvm adding this maybe I'm not worried about it", but if it's us in cg_llvm adding it then presumably it's worth trying to keep it working.)

Hmm, thinking more about what you said about the transmute, I wonder if this is something I regressed a long time ago with moving transmute to being primitive instead of always going through memory. Box<Box<i32>> and Box<i32> are both BackendAbi::Scalar, so maybe something is passed through a transmute now instead of being loaded from an alloca as the Box which would be where the !align on the load would be coming from...

I think that pushes me to just r=me for this stuff, as if it's an existing "oh this combination isn't great" that's not this PR's problem, and we can reoptimize it later as needed.

rustbot · 2026-02-17T10:51:15Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

oli-obk · 2026-02-17T10:52:13Z

tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.32bit.panic-abort.diff

+          _24 = copy _25 as *mut u8 (PtrToPtr);
+          StorageDead(_25);
+          _23 = copy _24 as std::ptr::NonNull<u8> (Transmute);
+-         StorageDead(_24);
+         nop;
+          _13 = copy _23 as *mut u8 (Transmute);


I think we need to teach gvn about skipping transmutes that go back and forth 😆

ah... we explicitly skip them with transmute_may_have_niche_of_interest_to_backend. Maybe we could unconditionally fold them, but also emit an assume?

Yeah, that's harder in GVN because GVN doesn't like updating things other than the value. (Also because of the fat-to-thin pointer cast. If fat pointers were just a struct things could be much easier here since part of the complication to folding is the transmute of the fat pointer.)

Musing: Maybe we always fold in GVN, but change the "remove unnecessary statements" code to emit an assume for an "unnecessary" transmute that creates a validity restriction? Dunno where the best place to do this would be.

(Of course, the other thing I would like is just a native "non-null with const-generic alignment" opaque pointer type. Whenever I see MIR needing to worry about *mut u8 vs *const u8 or about *mut u8 vs *mut () I get sad.)

rust-log-analyzer · 2026-02-17T11:42:17Z

The job x86_64-gnu-gcc failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)


=> Removing the following docker images:
WARNING: This output is designed for human readability. For machine-readable output, please use --format.
IMAGE                                               ID             DISK USAGE   CONTENT SIZE   EXTRA
ghcr.io/dependabot/dependabot-updater-core:latest   9a6a20114926       1.18GB          310MB        
=> Removing docker images...
Deleted Images:
untagged: ghcr.io/dependabot/dependabot-updater-core:latest
deleted: sha256:9a6a20114926442eeadab0732ddd7264ecafc907389c47974b1825d779571319

Total reclaimed space: 309.7MB

********************************************************************************
---
test [codegen-units] tests/codegen-units/item-collection/non-generic-functions.rs ... ok
test [codegen-units] tests/codegen-units/item-collection/static-init.rs ... ok
test [codegen-units] tests/codegen-units/item-collection/statics-and-consts.rs ... ok
test [codegen-units] tests/codegen-units/item-collection/overloaded-operators.rs ... ok
FATAL: !(missing.is_empty() && unexpected.is_empty() && wrong_cgus.is_empty())
test [codegen-units] tests/codegen-units/item-collection/opaque-return-impls.rs ... FAILED
test [codegen-units] tests/codegen-units/item-collection/trait-implementations.rs ... ok
test [codegen-units] tests/codegen-units/item-collection/trait-method-default-impl.rs ... ok
test [codegen-units] tests/codegen-units/item-collection/trait-method-as-argument.rs ... ok
test [codegen-units] tests/codegen-units/item-collection/transitive-drop-glue.rs ... ok
---
---- [codegen-units] tests/codegen-units/item-collection/opaque-return-impls.rs stdout ----

These items were contained but should not have been:

MONO_ITEM fn std::boxed::box_new_uninit @@ opaque_return_impls.a116a005f37f8ef5-cgu.0[Internal]



thread '[codegen-units] tests/codegen-units/item-collection/opaque-return-impls.rs' panicked at src/tools/compiletest/src/runtest/codegen_units.rs:111:13:
fatal error

For more information how to resolve CI failures of this job, visit this link.

rustbot assigned scottmcm Feb 16, 2026

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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 16, 2026

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 16, 2026

rust-bors bot pushed a commit that referenced this pull request Feb 16, 2026

Auto merge of #152702 - oli-obk:nonnulltransmute, r=<try>

ea5f33e

Prepare NonNull for pattern types

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 16, 2026

This comment has been minimized.

Sign in to view

scottmcm reviewed Feb 17, 2026

View reviewed changes

Prepare NonNull for pattern types

349c689

oli-obk force-pushed the nonnulltransmute branch from b11fecb to 349c689 Compare February 17, 2026 10:51

oli-obk commented Feb 17, 2026

View reviewed changes

Uh oh!

Conversation

oli-obk commented Feb 16, 2026

Uh oh!

oli-obk commented Feb 16, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Feb 16, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Feb 16, 2026

Overall result: ❌✅ regressions and improvements - please read the text below

Uh oh!

This comment has been minimized.

scottmcm left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Feb 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-log-analyzer commented Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

scottmcm left a comment •

edited by rustbot

Loading