add extract_bits and deposit_bits intrinsics by malezjaa · Pull Request #164876 · rust-lang/rust

malezjaa · 2026-10-24T21:55:26Z

Tracking issue: #149069

LLVM 23 introduced generic intrinsics for pext (extract_bits) and pdep (deposit_bits)

rustbot · 2026-10-24T21:55:29Z

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

Any special-casing of Miri in the standard library requires review.

cc @rust-lang/miri

⚠️ #[miri::intrinsic_fallback_is_spec] must only be used if the function actively checks for all UB cases,
and explores the possible non-determinism of the intrinsic.

cc @rust-lang/miri

rustbot · 2026-10-24T21:55:32Z

r? @JohnTitor

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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: libs
libs expanded to 37 candidates
Random selection from ChrisDenton, JohnTitor, Mark-Simulacrum, thomcc

RalfJung · 2026-10-25T11:17:04Z

Tests were made by AI, but I understand how they work.

FWIW our policy disallows code generation, and there is no exception for tests. For those you're supposed to have a pre-arranged reviewer who agreed to reviewing LLM code.

malezjaa · 2026-10-25T11:19:06Z

Oh right, I'll close this and open a new pr with tests made by me If that's okay

RalfJung · 2026-10-25T11:25:48Z

You can also update this PR. Or you can make it a draft and ask for a reviewer on Zulip. Whatever you prefer.

RalfJung · 2026-10-25T11:32:06Z

+#[unstable(feature = "uint_gather_scatter_bits", issue = "149069")]
+#[rustc_const_unstable(feature = "uint_gather_scatter_bits", issue = "149069")]
+#[miri::intrinsic_fallback_is_spec]
+pub const fn pext<T: [const] fallback::Pext>(x: T, mask: T) -> T {


These names are extremely opaque and make little sense to me.

Please name intrinsics more like the Rust operation than their often obscure LLVM name. You can mention the LLVM name in the docs.

View changes since the review

JohnTitor · 2026-11-03T11:01:25Z

r? RalfJung

rustbot · 2026-11-03T11:01:31Z

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

RalfJung

This seems reasonable, modulo some nits.
@rust-lang/wg-llvm any concerns about using these new LLVM intrinsics?

@rustbot author

View changes since this review

RalfJung · 2026-11-04T13:57:00Z

                let pair = self.insert_value(pair, high, 1);
                pair
            }
+            sym::deposit_bits | sym::extract_bits if llvm_version >= (23, 0, 0) => {


Why are we only using this for LLVM >= 23? Please add a comment.

RalfJung · 2026-11-04T13:58:23Z


            #[inline(always)]
-            pub(in crate::num) const fn extract_impl(mut x: $U, sparse: $U) -> $U {
+            pub(crate) const fn extract_impl(mut x: $U, sparse: $U) -> $U {


If these are now only used for the fallback impl, they should be moved to intrinsics/fallback.

RalfJung · 2026-11-04T13:59:43Z

Is this test doing anything useful? We already have the other test ensuring that the LLVM intrinsic gets generated.

RalfJung · 2026-11-04T14:02:50Z

+/// This corresponds to `@llvm.pext.*`.
+#[rustc_intrinsic]
+#[rustc_nounwind]
+#[unstable(feature = "uint_gather_scatter_bits", issue = "149069")]


Intrinsic will not be stabilized so they should not have a separate feature gate.

rustbot · 2026-11-04T14:04:13Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

nikic · 2026-11-04T14:11:43Z

@rust-lang/wg-llvm any concerns about using these new LLVM intrinsics?

Did you confirm that they produce good codegen, esp. on LLVM 23? For the case where they get expanded, that is.

RalfJung · 2026-11-04T14:18:22Z

For the case where they get expanded, that is.

I don't know what you mean by that.

nikic · 2026-11-04T14:31:14Z

When there is no native instruction

RalfJung · 2026-11-04T14:32:17Z

Which architectures would that affect?
There is an asm test but it runs on x86 and doesn't seem to do much.

nikic · 2026-11-04T14:47:08Z

You'd want to look at something like x86 without bmi2.

Just trying the 32-bit case, we have current Rust codegen: https://rust.godbolt.org/z/4snnWPhKP

And the LLVM intrinsic expansion: https://llvm.godbolt.org/z/bx6TKW6hf

It looks like Rust's current fallback produces less instructions, it's ~180 total vs ~250 total.

Doesn't look like this changes substantially in LLVM 24 either.

RalfJung · 2026-11-04T14:51:08Z

So... this PR is a regression for such targets?

Any suggestions for what to do? We could in principle gate the use of the intrinsic on target & target features, and use the fallback body otherwise, just like it is already used on older LLVM.

nikic · 2026-11-04T15:06:53Z

It looks like there is an LLVM PR to improve the expansion: llvm/llvm-project#227292

I'd probably just wait until that gets in.

rustbot · 2026-11-05T16:14:03Z

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.

malezjaa · 2026-11-05T16:15:02Z

@rustbot ready

rustbot · 2026-11-05T16:15:06Z

Error: shortcut handler unexpectedly failed in this comment: failed to remove Label { name: "S-waiting-on-author" }

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

RalfJung

This LGTM from the compiler implementation side, but I cannot comment on when we should actually start using the attribute wrt LLVM support.

r? @nikic

View changes since this review

RalfJung · 2026-11-05T19:53:32Z

                let pair = self.insert_value(pair, high, 1);
                pair
            }
+            // FIXME(llvm): These intrinsics were introduced in LLVM 23.


Suggested change

// FIXME(llvm): These intrinsics were introduced in LLVM 23.

// Checking LLVM version as these intrinsics were introduced in LLVM 23.

RalfJung · 2026-11-05T19:57:51Z

+#[unstable(feature = "core_intrinsics", issue = "none")]
+#[rustc_const_unstable(feature = "core_intrinsics", issue = "none")]


You don't need #[unstable] here, the const attribute is enough.

RalfJung · 2026-11-05T20:06:24Z

r? @nikic

rustbot · 2026-11-05T20:06:28Z

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

malezjaa · 2026-11-05T20:13:11Z

Updated. Thank you for you review :D

rust-bors · 2026-11-10T20:31:23Z

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

rustbot assigned JohnTitor Oct 24, 2026

This comment has been minimized.

Sign in to view

RalfJung added the llm-assisted An LLM-assisted PR as defined by the LLM policy. Requires ahead-of-time consent by assignee. label Oct 25, 2026

malezjaa closed this Oct 25, 2026

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 25, 2026

malezjaa reopened this Oct 25, 2026

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 25, 2026

RalfJung reviewed Oct 25, 2026

View reviewed changes

malezjaa force-pushed the add-pext-and-pdep-intrinsics branch from b457daf to 50266b1 Compare October 25, 2026 12:28

malezjaa changed the title ~~add pext and pdep intrinsics~~ add extract_bits and deposit_bits intrinsics Oct 25, 2026

This comment has been minimized.

Sign in to view

malezjaa force-pushed the add-pext-and-pdep-intrinsics branch from 50266b1 to 3e22e48 Compare October 25, 2026 13:16

rustbot assigned RalfJung and unassigned JohnTitor Nov 3, 2026

RalfJung reviewed Nov 4, 2026

View reviewed changes

rustbot 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 Nov 4, 2026

malezjaa force-pushed the add-pext-and-pdep-intrinsics branch 2 times, most recently from 8d64e63 to 935f428 Compare November 5, 2026 16:13

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 5, 2026

This comment has been minimized.

Sign in to view

malezjaa force-pushed the add-pext-and-pdep-intrinsics branch from 935f428 to 3ead23f Compare November 5, 2026 16:23

RalfJung reviewed Nov 5, 2026

View reviewed changes

rustbot assigned nikic and unassigned RalfJung Nov 5, 2026

add extract and deposit bits intrinsics

375326e

malezjaa force-pushed the add-pext-and-pdep-intrinsics branch from 3ead23f to 375326e Compare November 5, 2026 20:12

	// FIXME(llvm): These intrinsics were introduced in LLVM 23.
	// Checking LLVM version as these intrinsics were introduced in LLVM 23.

		#[unstable(feature = "core_intrinsics", issue = "none")]
		#[rustc_const_unstable(feature = "core_intrinsics", issue = "none")]

Conversation

malezjaa commented Oct 24, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 24, 2026

Uh oh!

rustbot commented Oct 24, 2026

Uh oh!

This comment has been minimized.

RalfJung commented Oct 25, 2026

Uh oh!

malezjaa commented Oct 25, 2026

Uh oh!

RalfJung commented Oct 25, 2026

Uh oh!

RalfJung Oct 25, 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!

This comment has been minimized.

JohnTitor commented Nov 3, 2026

Uh oh!

rustbot commented Nov 3, 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!

RalfJung Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Nov 4, 2026

Uh oh!

nikic commented Nov 4, 2026

Uh oh!

RalfJung commented Nov 4, 2026

Uh oh!

nikic commented Nov 4, 2026

Uh oh!

RalfJung commented Nov 4, 2026

Uh oh!

nikic commented Nov 4, 2026

Uh oh!

RalfJung commented Nov 4, 2026

Uh oh!

nikic commented Nov 4, 2026

Uh oh!

rustbot commented Nov 5, 2026

Uh oh!

malezjaa commented Nov 5, 2026

Uh oh!

rustbot commented Nov 5, 2026

Uh oh!

This comment has been minimized.

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!

RalfJung Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Nov 5, 2026

Uh oh!

rustbot commented Nov 5, 2026

Uh oh!

malezjaa commented Nov 5, 2026

Uh oh!

rust-bors Bot commented Nov 10, 2026

malezjaa commented Oct 24, 2026 •

edited by rustbot

Loading

RalfJung Oct 25, 2026 •

edited by rustbot

Loading

RalfJung left a comment •

edited by rustbot

Loading

RalfJung left a comment •

edited by rustbot

Loading