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.

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

Support 128-bit atomics on x86_64-fortanix-unknown-sgx #130552

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Sep 19, 2024

Based on my comment in #99069 (comment):

If my understanding about the SGX target is correct, this is a target that requires Intel chip-specific technology, so I suspect that 128-bit atomics is always available (because all Intel x86_64 chips supports cmpxchg16b).

cc @jethrogb: Is it okay to have the above assumptions for this target? (Or is this target intentionally not currently enabling cx16 for some reason?)

@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2024

r? @wesleywiser

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

@rustbot 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 Sep 19, 2024
@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2024

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

@jieyouxu jieyouxu added O-SGX Target: SGX A-atomic Area: Atomics, barriers, and sync primitives labels Sep 19, 2024
@jethrogb
Copy link
Contributor

Yes SGX is generally considered Intel-specific. If you could point to where in the Intel documentation it talks about 128-bit atomics always being supported, that would be helpful.

@taiki-e
Copy link
Member Author

taiki-e commented Sep 19, 2024

If you could point to where in the Intel documentation it talks about 128-bit atomics always being supported, that would be helpful.

Sorry, I do not know of any Intel documentation that explains this.

I mainly referred to the following three resources:

@taiki-e
Copy link
Member Author

taiki-e commented Sep 19, 2024

If you could point to where in the Intel documentation it talks about 128-bit atomics always being supported, that would be helpful.

Sorry, I do not know of any Intel documentation that explains this.

Ah, "CMPXCHG8B/CMPXCHG16B—Compare and Exchange Bytes" section in Intel 64 and IA-32 Architectures Software Developer’s Manual says CMPXCHG16B in 64-Bit Mode is "Valid".

(When it is not available on older CPUs, such as CMPXCHG8B/CMPXCHG in Compat/Leg Mode, it has annotation about compatibility -- they are "Valid*" not "Valid" and have "IA-32 Architecture Compatibility" sections that say "not supported on Intel processors earlier than ...".)

EDIT: I missed exceptions sections #130552 (comment)

@jethrogb
Copy link
Contributor

jethrogb commented Sep 19, 2024

Just the “valid” note is not sufficient. In the instruction description under 64-mode exceptions, it says:

#UD If CPUID.01H:ECX.CMPXCHG16B[bit 13] = 0.

Which indicates that CPU support is not guaranteed in general.

@wesleywiser wesleywiser 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 Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-atomic Area: Atomics, barriers, and sync primitives O-SGX Target: SGX S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants