-
Notifications
You must be signed in to change notification settings - Fork 300
Make cpuid safe and update docs
#1935
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
r? @folkertdev rustbot has assigned @folkertdev. Use |
|
Proposing FCP on this (in fact, even nominating it) is blocked on: |
|
@sayantn, if you could please update the PR description with the details of the stabilization being proposed, the motivation, etc., that would be helpful to us in considering this nomination and for proposing FCP. |
|
👍 for this. One clarification:
For the avoidance of ambiguity: lower than i586, which is lower than the current 32-bit x86 targets in Rust. |
|
Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
|
We discussed this in the lang team meeting today and the agreement was that if we add support for an older target like i586, we can make this unsafe and require target_feature for the intrinsic on that target. These intrinsics are already platform-specific, so it's natural to have a more target-specific definition that changes the safety of the intrinsic. @rfcbot reviewed |
|
@rfcbot reviewed |
Counterproposal to rust-lang/rust#146560
This makes
__cpuid,__cpuid_countand__get_cpuid_maxsafe to use. All of these just call the CPUID instruction, so it is safe to call whenever the CPUID instruction is available (it has defined behavior even if unsupported leaf values are passes). There are cases when it might not be availablehas_cpuidfunction was removed with this exact motivation, and it is justifiedAs these are very niche cases we propose to make cpuid safe in general.
Alternatives
Leave it unsafe
This is always an option. But for most use cases using
__cpuidis perfectly safe, so it is inconvenient (and more so for projects that ban uses ofunsafe).Introduce a
cpuidtarget feature, and take advantage oftarget_feature_11This is what rust-lang/rust#146560 does. As all X86 target features (other than
soft_floatandx87) were introduced after CPUID, we can add a detection-only target featurecpuidthat is implied by all X86 target features. This makes using it ini686orx86_64targets safe due tossebeing auto-enabled in the targe description, whereasi386remains unsafe as it doesn't enablesse