Replace NullOp::SizeOf and NullOp::AlignOf by lang items. #147793

cjgillot · 2025-10-16T22:24:36Z

Fixes #119729
Keeps #136175 as it involves offset_of! which this PR does not touch.

cjgillot · 2025-10-16T22:33:04Z

Replace NullOp::SizeOf and NullOp::AlignOf by lang items.

rust-bors · 2025-10-17T00:45:10Z

☀️ Try build successful (CI)
Build commit: 2cefd8f (2cefd8ff4961f18771f6f840878942cbfbc03afe, parent: 53a741fc4b8cf2d8e7b1b2336ed8edf889db84f4)

rust-timer · 2025-10-17T01:54:54Z

Finished benchmarking commit (2cefd8f): 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)	0.5%	[0.4%, 0.6%]	3
Regressions ❌ (secondary)	0.7%	[0.5%, 0.7%]	4
Improvements ✅ (primary)	-0.2%	[-0.2%, -0.1%]	2
Improvements ✅ (secondary)	-0.2%	[-0.7%, -0.1%]	14
All ❌✅ (primary)	0.2%	[-0.2%, 0.6%]	5

Max RSS (memory usage)

Results (primary -0.3%, secondary -1.4%)

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

	mean	range	count
Regressions ❌ (primary)	1.5%	[0.4%, 3.0%]	8
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.4%	[-7.6%, -1.1%]	5
Improvements ✅ (secondary)	-1.4%	[-2.2%, -0.8%]	3
All ❌✅ (primary)	-0.3%	[-7.6%, 3.0%]	13

Cycles

Results (primary -2.9%, secondary 6.8%)

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

Binary size

Results (primary -0.0%, secondary -0.3%)

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

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.5%]	22
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	11
Improvements ✅ (primary)	-0.2%	[-0.8%, -0.0%]	18
Improvements ✅ (secondary)	-1.2%	[-3.2%, -0.3%]	4
All ❌✅ (primary)	-0.0%	[-0.8%, 0.5%]	40

Bootstrap: 475.105s -> 474.369s (-0.15%)
Artifact size: 390.35 MiB -> 390.39 MiB (0.01%)

library/core/src/mem/mod.rs

compiler/rustc_const_eval/src/interpret/intrinsics.rs

library/core/src/mem/mod.rs

rustbot · 2025-10-19T23:34:44Z

This PR was rebased onto a different master 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.

cjgillot · 2025-10-19T23:51:08Z

@bors try @rust-timer queue

Replace NullOp::SizeOf and NullOp::AlignOf by lang items.

rust-bors · 2025-10-20T02:01:35Z

☀️ Try build successful (CI)
Build commit: ab65cea (ab65cead52dcf8e64ec14319f1c77892296050e5, parent: f04e3dfc87d7e2b6ad53e7a52253812cd62eba50)

rust-timer · 2025-10-20T03:56:06Z

Finished benchmarking commit (ab65cea): 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)	0.3%	[0.3%, 0.4%]	2
Regressions ❌ (secondary)	0.3%	[0.1%, 0.7%]	13
Improvements ✅ (primary)	-0.5%	[-1.0%, -0.2%]	4
Improvements ✅ (secondary)	-0.1%	[-0.3%, -0.0%]	5
All ❌✅ (primary)	-0.2%	[-1.0%, 0.4%]	6

Max RSS (memory usage)

Results (primary -1.3%, secondary -2.9%)

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

	mean	range	count
Regressions ❌ (primary)	1.1%	[0.5%, 2.2%]	3
Regressions ❌ (secondary)	3.2%	[3.2%, 3.2%]	2
Improvements ✅ (primary)	-2.6%	[-4.7%, -1.5%]	6
Improvements ✅ (secondary)	-3.9%	[-5.7%, -1.3%]	12
All ❌✅ (primary)	-1.3%	[-4.7%, 2.2%]	9

Cycles

Results (secondary 1.7%)

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)	5.4%	[2.6%, 8.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.5%	[-6.9%, -3.0%]	3
All ❌✅ (primary)	-	-	0

Binary size

Results (primary -0.0%, 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.1%	[0.0%, 0.4%]	20
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	9
Improvements ✅ (primary)	-0.1%	[-0.8%, -0.0%]	19
Improvements ✅ (secondary)	-1.2%	[-3.1%, -0.3%]	4
All ❌✅ (primary)	-0.0%	[-0.8%, 0.4%]	39

Bootstrap: 473.871s -> 473.854s (-0.00%)
Artifact size: 388.68 MiB -> 388.39 MiB (-0.08%)

RalfJung · 2025-10-20T06:39:26Z

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

    error: InterpErrorInfo<'tcx>,
 ) -> ErrorHandled {
    let (error, backtrace) = error.into_parts();
-    backtrace.print_backtrace();


This one here should stay -- it's part of how RUST_CTFE_BACKTRACE works. By default this is a fairly trivial NOP.

oli-obk · 2025-10-20T07:32:57Z

Instead of having the intrinsics const evaluated and requiring an actual body for the consts, we could also hijack const eval of these lang items directly and thus never have to do the work of actually evaluating their body.

RalfJung · 2025-10-20T08:32:49Z

I find the intrinsics route less confusing: - intrinsics for when the library needs to call into the compiler - lang items for when the compiler needs to call into the library However, if it makes a big enough perf difference, we could deviate from that pattern.

cjgillot · 2025-10-20T23:34:08Z

The perf regression is very small, sub-percent, does this warrant extra investigation?

saethlin · 2025-10-20T23:38:43Z

I do not think so. Results on real crates are net green.

cjgillot · 2025-10-21T00:37:55Z

r? @oli-obk if you've got time, or reassign

rustbot · 2025-10-21T00:37:59Z

oli-obk is not on the review rotation at the moment.
They may take a while to respond.

scottmcm · 2025-10-21T17:29:59Z

compiler/rustc_codegen_cranelift/example/mini_core.rs

 }

+pub const fn size_of<T>() -> usize {
+    const { intrinsics::size_of::<T>() }


no idea if it matters, but should this be

Suggested change

const { intrinsics::size_of::<T>() }

<T as SizedTypeProperties>::SIZE

here too? (And ditto for align.)

scottmcm · 2025-10-21T18:04:26Z

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

                let ty = self.monomorphize(ty);
                let layout = bx.cx().layout_of(ty);
                let val = match null_op {
-                    mir::NullOp::SizeOf => {


(No action) I do like that this means the different backends don't need this any more, which is nice since it all needs to match what CTFE did anyway 👍

scottmcm · 2025-10-21T18:09:16Z

tests/mir-opt/box_expr.main.ElaborateDrops.diff

-          _2 = SizeOf(S);
-          _3 = AlignOf(S);
-          _4 = alloc::alloc::exchange_malloc(move _2, move _3) -> [return: bb1, unwind continue];
+          _2 = alloc::alloc::exchange_malloc(const <S as std::mem::SizedTypeProperties>::SIZE, const <S as std::mem::SizedTypeProperties>::ALIGN) -> [return: bb1, unwind continue];


Lovely to see this inline and no longer need the locals 👍

scottmcm · 2025-10-21T18:15:20Z

tests/mir-opt/const_prop/boxes.main.GVN.panic-abort.diff

-+         _4 = const 4_usize;
-+         _5 = const 4_usize;
-+         _6 = alloc::alloc::exchange_malloc(const 4_usize, const 4_usize) -> [return: bb1, unwind unreachable];
+          _4 = alloc::alloc::exchange_malloc(const <i32 as std::mem::SizedTypeProperties>::SIZE, const <i32 as std::mem::SizedTypeProperties>::ALIGN) -> [return: bb1, unwind unreachable];


Just confirming: the reason this is still the const, rather than 4_usize is that this is a GVN unit test? Some other pass is going to evaluate it in the normal flow, right?

Or does it not matter because anything asking for try_eval_target_usize on the const will get the value either way?

Oh, that's because GVN avoids to evaluate constants in-place. That does not really matter, any code that needs to evaluate it will easily. If we need it, it's a 3-line change in GVN, but I'd rather do it in a separate PR.

scottmcm · 2025-10-21T18:32:59Z

compiler/rustc_mir_transform/src/check_alignment.rs

+    let align_def_id = tcx.require_lang_item(LangItem::AlignOf, source_info.span);
+    let align_const =
+        Const::from_unevaluated(tcx, align_def_id).instantiate(tcx, &[pointee_ty.into()]);
+    let alignment = Operand::Constant(Box::new(ConstOperand {
+        span: source_info.span,
+        user_ty: None,
+        const_: align_const,
+    }));


ymmv: since this chunk (or quite similar) happens in a couple places now, would it be worth adding, say mir::Operand::size_of + mir::Operand::align_of? (Looks like it probably could have both of them call a private helper taking LangItem, or something.)

I think that'd both be easier for the next people who need it to find, as well as more obvious to read in the places that just want the size/align in the code.

scottmcm · 2025-10-21T18:35:26Z

compiler/rustc_public/src/mir/body.rs

-    /// Returns the size of a value of that type.
-    SizeOf,
-    /// Returns the minimum alignment of a type.
-    AlignOf,


FYI @celinval

scottmcm

I left some comments along the way, but this looks good to me. r=me with minor updates to address things if you see fit. (Or wait for oli if you think this should go through someone more knowledgeable about const stuff.)

View changes since this review

This comment has been minimized.

Sign in to view

cjgillot force-pushed the no-null-op branch from 5ca7d48 to b335a91 Compare October 16, 2025 22:32

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 Oct 16, 2025

rust-bors bot added a commit that referenced this pull request Oct 16, 2025

Auto merge of #147793 - cjgillot:no-null-op, r=<try>

2cefd8f

Replace NullOp::SizeOf and NullOp::AlignOf by lang items.

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 Oct 17, 2025

cjgillot force-pushed the no-null-op branch from b335a91 to a785cc9 Compare October 17, 2025 01:57

rustbot added the T-clippy Relevant to the Clippy team. label Oct 17, 2025

This comment has been minimized.

Sign in to view

cjgillot force-pushed the no-null-op branch from a785cc9 to a70daae Compare October 17, 2025 02:36

This comment has been minimized.

Sign in to view

RalfJung reviewed Oct 17, 2025

View reviewed changes

library/core/src/mem/mod.rs Outdated Show resolved Hide resolved

RalfJung reviewed Oct 17, 2025

View reviewed changes

compiler/rustc_const_eval/src/interpret/intrinsics.rs Outdated Show resolved Hide resolved

RalfJung reviewed Oct 17, 2025

View reviewed changes

library/core/src/mem/mod.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

cjgillot force-pushed the no-null-op branch 2 times, most recently from fca4c69 to 27154a0 Compare October 17, 2025 17:59

This comment has been minimized.

Sign in to view

cjgillot added 2 commits October 19, 2025 21:40

Bless miri.

9cf34ab

Adapt for 32bit.

35a6b51

cjgillot force-pushed the no-null-op branch from fa2dedc to 3d87bc1 Compare October 19, 2025 23:34

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 19, 2025

Auto merge of #147793 - cjgillot:no-null-op, r=<try>

ab65cea

Replace NullOp::SizeOf and NullOp::AlignOf by lang items.

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 19, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 20, 2025

RalfJung reviewed Oct 20, 2025

View reviewed changes

Reduce work to report errors.

001f9d2

cjgillot force-pushed the no-null-op branch from 3d87bc1 to 001f9d2 Compare October 20, 2025 23:29

rustbot assigned oli-obk Oct 21, 2025

scottmcm reviewed Oct 21, 2025

View reviewed changes

scottmcm approved these changes Oct 21, 2025

View reviewed changes

	const { intrinsics::size_of::<T>() }
	<T as SizedTypeProperties>::SIZE

Replace NullOp::SizeOf and NullOp::AlignOf by lang items. #147793

Are you sure you want to change the base?

Replace NullOp::SizeOf and NullOp::AlignOf by lang items. #147793

Conversation

cjgillot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

cjgillot commented Oct 16, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Oct 17, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Oct 17, 2025

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

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Oct 19, 2025

Uh oh!

cjgillot commented Oct 19, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Oct 20, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Oct 20, 2025

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Oct 20, 2025

Uh oh!

RalfJung commented Oct 20, 2025 via email

Uh oh!

cjgillot commented Oct 20, 2025

Uh oh!

saethlin commented Oct 20, 2025

Uh oh!

cjgillot commented Oct 21, 2025

Uh oh!

rustbot commented Oct 21, 2025

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

cjgillot commented Oct 16, 2025 •

edited

Loading

scottmcm left a comment •

edited by rustbot

Loading