Skip to content

Conversation

bal-e
Copy link

@bal-e bal-e commented Oct 22, 2025

Crates like raw_cpuid use core::arch::x86_64::__cpuid_count() to determine x86 CPU information. It's great that core provides such a function, instead of having to write inline assembly everywhere; but core's implementation does not use the asm! attributes pure and nomem. This means that calls to __cpuid_count() can't be elided or deduplicated. I'm writing some target-feature enhancement code (akin to multiversion), and I'd like to rely on CPUID getting optimized away appropriately.

The lack of these attributes was probably an oversight; I looked through the Git history and couldn't see any discussion of the purity of the instruction. While CPUID can have some less-than-pure effects (it acts like a strong memory fence), that's not the primary use case for it -- users who explicitly rely on those effects should write inline assembly where they can be more explicit about it.

In a future where CPUID is an LLVM intrinsic, this wouldn't matter, and LLVM would presumably be able to determine the purity of the intrinsic automatically. But until then, marking it as pure would be a nice optimization hint.

If there are concerns that users may wish to rely on the mild side effects of CPUID, or if there are genuine side effects I wasn't aware of, perhaps a __pure_cpuid_count() instruction could be added. I think it's better to make __cpuid_count() pure, but I'll leave that decision to the reviewer.

'__cpuid_count()' is implemented using inline assembly, because LLVM
doesn't have an intrinsic for it.  It's a pure operation, but this
wasn't marked in the 'asm!' invocation; so calls to it couldn't be
elided or deduplicated.  This change makes it pure.

CPUID does have _some_ less-than-pure effects -- e.g. it can be used as
a serializing instruction (like a strong memory fence).  Users who want
to rely on that could use inline assembly themselves instead.
@rustbot
Copy link
Collaborator

rustbot commented Oct 22, 2025

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

cc @Amanieu, @folkertdev, @sayantn

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 22, 2025

r? @joboet

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

@sayantn
Copy link
Contributor

sayantn commented Oct 22, 2025

cpuid shouldn't be pure, it is also a serializing operation, so it can't be reordered. I think it can be nomem though. cc @Amanieu

Also, it would be better if you open this pr in the rust-lang/stdarch repo

@bal-e
Copy link
Author

bal-e commented Oct 22, 2025

@sayantn: I'm aware it's a serializing instruction, but that's not the primary use case for this function. Maybe we could split the function into a pure and impure version? (__pure_cpuid_count() + __cpuid_count() or __impure_cpuid_count() + __cpuid_count()).

I'll refile this PR on stdarch and copy this comment over.

@sayantn
Copy link
Contributor

sayantn commented Oct 22, 2025

That sounds reasonable, and t-libs might wanna look at it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants