Don't compute FnAbi for LLVM intrinsics by bjorn3 · Pull Request #160077 · rust-lang/rust

bjorn3 · 2026-07-28T14:12:56Z

They don't have a sensible FnAbi, so the fact that we still compute an FnAbi for them requires us to make the ABI sanity check more lenient than it should be.

r? @RalfJung as all non-trivial changes are in Miri

rustbot · 2026-07-28T14:12:59Z

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

rustbot · 2026-07-28T14:13:02Z

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

bjorn3 · 2026-07-28T14:26:38Z

I guess this finishes my quest of getting rid of PassMode::Direct for BackendRepr::Memory that spanned over 5 years and 13 PRs:

There is still cleanup that can be done, but at least this wildly wrong combination is gone.

RalfJung · 2026-07-28T18:24:45Z

+    func: OpTy<'tcx, M::Provenance>,
    callee: FnVal<'tcx, M::ExtraFnVal>,


This introduces redundancy (func and callee conceptually contain the same data). So I am not a fan of this.

What's the rationale for these non-trivial interpreter changes? I would understand making the fn_abi field an Option for calls that don't need an ABI, but this goes much beyond that it seems, by entirely removing fn_abi (and fn_sig).

View changes since the review

I could replace the func field with just the Ty. Making fn_abi optional would mean I did have to match on the instance inside eval_callee_and_args and do an unwrap outside of it. I can do it if you prefer it that way.

I could replace the func field with just the Ty

That would be better, albeit still a bit odd.

Making fn_abi optional would mean I did have to match on the instance inside eval_callee_and_args and do an unwrap outside of it. I can do it if you prefer it that way.

Well I don't even know yet why you need to change all those other things. I could reverse engineer it from your PR but it'd be easier if you just explained it to me.

The reason I need this change is because I made calculating the FnAbi for LLVM intrinsics panic and eval_callee_and_args is also called for LLVM intrinsics, so eval_callee_and_args must avoid computing the FnAbi for LLVM intrinsics at least.

Making fn_abi optional would mean I did have to match on the instance inside eval_callee_and_args and do an unwrap outside of it. I can do it if you prefer it that way.

I feel like that would result in a much smaller diff?
Maybe it could even be an Either<FnAbi, T> where T is whatever information we need to invoke the intrinsic (DefId?).

Making fn_abi optional would mean I did have to match on the instance inside eval_callee_and_args and do an unwrap outside of it. I can do it if you prefer it that way.

Wasn't too bad actually. Does it look fine to you?

bjorn3 · 2026-07-29T10:57:35Z

-    fn test_f32x2(a: f32x2);
-    fn test_f32x2_arr(a: f32x2);
-    fn test_simd(a: Simd<i32, 4>);
-    fn test_simd_unaligned(a: Simd<i32, 3>);


There is no way to test this specific case anymore it seems. LLVM doesn't accept PackedSimd on intrinsics, extern "unadjusted" requires LLVM intrinsics and any other ABI doesn't pass non-power-of-2 vectors as { [3 x i32] }.

View changes since the review

bjorn3 · 2026-07-31T12:21:54Z

Looks like compiler-builtins still has some extern "unadjusted" definitions. Will need to replace those.

rustbot · 2026-07-31T13:33:50Z

compiler-builtins is developed in its own repository. If possible, consider making this change to rust-lang/compiler-builtins instead.

cc @tgross35

RalfJung

interpreter changes mostly LGTM.

compiler-builtins changes will need a review by @tgross35 .

View changes since this review

RalfJung · 2026-08-01T08:35:01Z

            }
            ty::FnDef(def_id, args) => {
                let instance = self.resolve(def_id, args.no_bound_vars().unwrap())?;
+                // Don't compute FnAbi for LLVM intrinsics. Trying to that would panic.


Suggested change

// Don't compute FnAbi for LLVM intrinsics. Trying to that would panic.

// Don't compute FnAbi for LLVM intrinsics. Trying to do that would panic.

// Rust intrinsics however *do* need a FnAbi as we may invoke the

// fallback body like a regular function.

RalfJung · 2026-08-01T08:40:22Z

+                        @ ty::Instance { def: ty::InstanceKind::LlvmIntrinsic(_), args: _ },
+                    ) => {
+                        // FIXME: Should `InPlace` arguments be reset to uninit?
+                        M::call_llvm_intrinsic(


init_fn_call already has an LlvmIntrinsic case, this seems to duplicate that?

You can make init_fn_call take an Option<&FnAbi> as well.

RalfJung · 2026-08-01T08:41:40Z

                DUMMY_SP,
            );
+            if let InstanceKind::LlvmIntrinsic(..) = instance.def {
+                return;


Suggested change

return;

// LLVM intrinsics don't have an ABI, so there is nothing to check.

return;

RalfJung · 2026-08-01T08:43:19Z

+        abi,
+        ExternAbi::Unadjusted,
+        "fn_abi_of_instance should not be called on LLVM intrinsics"
+    );


It's kind of annoying that this panics and hence everyone has to carefully tiptoe around it. Can't we treat this more like Rust intrinsics where we can compute an ABI but it just doesn't matter?

RalfJung · 2026-08-01T08:45:12Z

@@ -1,19 +1,21 @@
 // Make sure that no 0-sized padding is inserted in structs and that
 // structs are represented as expected by Neon intrinsics in LLVM.
 // See #87254.
+//@ only-aarch64


This test is now impossible to run for the majority of contributors (that are on x86).

Please remove the only and add --target and minicore so everyone can run the test.

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 Jul 28, 2026

rustbot assigned RalfJung Jul 28, 2026

This comment has been minimized.

Sign in to view

RalfJung reviewed Jul 28, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

bjorn3 commented Jul 29, 2026

View reviewed changes

bjorn3 force-pushed the no_unadjusted_fn_abi branch from 7358ce7 to eacedff Compare July 29, 2026 10:58

bjorn3 added 3 commits July 31, 2026 14:02

Don't use extern "unadjusted" outside of LLVM intrinsics in tests

35f9fa2

Avoid computing FnAbi for llvm intrinsics in miri

707fca5

Don't compute FnAbi for LLVM intrinsics

fd68ce3

Reduce const eval diff

e2873a0

bjorn3 force-pushed the no_unadjusted_fn_abi branch 2 times, most recently from ef2938a to e2873a0 Compare July 31, 2026 12:28

Avoid extern "unadjusted" in compiler-builtins

e848978

rustbot added the A-compiler-builtins Area: compiler-builtins (https://github.com/rust-lang/compiler-builtins) label Jul 31, 2026

RalfJung reviewed Aug 1, 2026

View reviewed changes

		func: OpTy<'tcx, M::Provenance>,
		callee: FnVal<'tcx, M::ExtraFnVal>,

-                // Don't compute FnAbi for LLVM intrinsics. Trying to that would panic.
+                // Don't compute FnAbi for LLVM intrinsics. Trying to do that would panic.
+                // Rust intrinsics however *do* need a FnAbi as we may invoke the
+                // fallback body like a regular function.

	return;
	// LLVM intrinsics don't have an ABI, so there is nothing to check.
	return;

Uh oh!

Uh oh!

Conversation

bjorn3 commented Jul 28, 2026

Uh oh!

rustbot commented Jul 28, 2026

Uh oh!

rustbot commented Jul 28, 2026

Uh oh!

bjorn3 commented Jul 28, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

RalfJung Jul 28, 2026 • 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!

RalfJung Jul 29, 2026 • edited 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!

This comment has been minimized.

bjorn3 Jul 29, 2026 • 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!

bjorn3 commented Jul 31, 2026

Uh oh!

rustbot commented Jul 31, 2026

Uh oh!

RalfJung 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RalfJung Jul 28, 2026 •

edited by rustbot

Loading

RalfJung Jul 29, 2026 •

edited

Loading

bjorn3 Jul 29, 2026 •

edited by rustbot

Loading

RalfJung left a comment •

edited by rustbot

Loading