Skip to content
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.

Sign up for GitHub

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

Enable CMPXCHG16B, SSE3, SAHF/LAHF and 128-bit Atomics (in nightly) in Windows x64 #120820

Merged
merged 7 commits into from
Feb 29, 2024
+11 −6
Merged

Enable CMPXCHG16B, SSE3, SAHF/LAHF and 128-bit Atomics (in nightly) in Windows x64 #120820

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d51e703
As Windows 10 requires certain features like CMPXCHG16B and a few oth…
CKingX Feb 9, 2024
d6766e2
Update x86_64_pc_windows_msvc.rs
CKingX Feb 9, 2024
fcb06f7
Update x86_64_pc_windows_msvc.rs
CKingX Feb 9, 2024
abeac8f
Update x86_64_uwp_windows_gnu.rs
CKingX Feb 9, 2024
1c6dda7
Possibly removed merge policy
CKingX Feb 9, 2024
376c7b9
Added sahf feature to windows targets
CKingX Feb 13, 2024
2d25c3b
Updated test to account for added previous features (thanks erikdesja…
CKingX Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::spec::{base, SanitizerSet, Target};
pub fn target() -> Target {
let mut base = base::windows_msvc::opts();
base.cpu = "x86-64".into();
base.features = "+cmpxchg16b,+sse3".into();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+sahf too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That I am not sure. x86-64-v2 onwards don't use sahf though it is listed in the features. CMPXCHG16B and SSE3 are listed. Does that mean Rustc doesn't yet make use of these instructions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you are seeing but x86-64-v2 totally does specify SAHF:
https://github.com/llvm/llvm-project/blob/a9700904765590ca2fbf08c0cc36d0da1107d3a7/llvm/lib/Target/X86/X86.td#L808-L809

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used this

rustc --print=cfg -C target-cpu=x86-64-v2
debug_assertions
panic="unwind"
target_arch="x86_64"
target_endian="little"
target_env="msvc"
target_family="windows"
target_feature="cmpxchg16b"
target_feature="fxsr"
target_feature="popcnt"
target_feature="sse"
target_feature="sse2"
target_feature="sse3"
target_feature="sse4.1"
target_feature="sse4.2"
target_feature="ssse3"
target_has_atomic="16"
target_has_atomic="32"
target_has_atomic="64"
target_has_atomic="8"
target_has_atomic="ptr"
target_os="windows"
target_pointer_width="64"
target_vendor="pc"
windows

Did some more digging and turns out SAHF seems to be for kernel rather than userspace code. Hence it is not listed in the rustc command I ran above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also seems to be tied to virtualization and for kernel space.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've been researching this a bit more. On balance I do think we should add +sahf. While I don't think it's that important, it was seen fit to be included in x86-64-v2 feature level and I think we should align ourselves with that (although obviously dropping the SSE level back down to 3).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

base.plt_by_default = false;
base.max_atomic_width = Some(64);
base.supported_sanitizers = SanitizerSet::ADDRESS;
Expand Down
Loading