Prefer asm! in std - all in sgx module #77292
Conversation
library/std/src/sys/sgx/abi/mem.rs
Outdated
| asm!( | ||
| "lea IMAGE_BASE(%rip), {}", | ||
| lateout(reg) base, | ||
| // FIXME(#76738): ATT syntax is used to support LLVM 8 and 9. |
There was a problem hiding this comment.
This doesn't require a FIXME (same for other similar comments throughout the PR)
There was a problem hiding this comment.
Fellow contributors may not know this and try to convert to intel syntax.
There was a problem hiding this comment.
Ok but FIXME implies this needs to be changed, which is not the case.
There was a problem hiding this comment.
It could be change in the future when #76738 resolved.
But I could change it to a NOTE if you prefer.
library/std/src/sys/sgx/abi/mem.rs
Outdated
| "lea IMAGE_BASE(%rip), {}", | ||
| lateout(reg) base, | ||
| // FIXME(#76738): ATT syntax is used to support LLVM 8 and 9. | ||
| options(att_syntax, nostack), |
There was a problem hiding this comment.
Also: preserves_flags, nomem.
Does nomem imply nostack?
There was a problem hiding this comment.
nostack is separate from nomem: you can still allocate and use stack memory with nomem as long as you don't access any memory outside the asm.
library/std/src/sys/sgx/ext/arch.rs
Outdated
| in("rbx") request, | ||
| in("rcx") out.as_mut_ptr(), | ||
| // FIXME(#76738): ATT syntax is used to support LLVM 8 and 9. | ||
| options(att_syntax, preserves_flags), |
There was a problem hiding this comment.
Certainly not preserves_flags. But you can use nostack.
library/std/src/sys/sgx/ext/arch.rs
Outdated
| in("rcx") reportdata, | ||
| in("rdx") report.as_mut_ptr(), | ||
| // FIXME(#76738): ATT syntax is used to support LLVM 8 and 9. | ||
| options(att_syntax), |
There was a problem hiding this comment.
Also: preserves_flags, nostack.
|
Passed CI, just fall at upload state, which it should be. |
|
LGTM |
|
@bors r+ |
|
📌 Commit d477201 has been approved by |
bors
added
the
S-waiting-on-bors
Prefer asm! in std - all in sgx module Similar to the change in rust-lang#76669 but all `llvm_asm!` is gate in x86/x86_64 target. Godbolt: - https://rust.godbolt.org/z/h7nG1h - https://rust.godbolt.org/z/xx39hW r? @ghost
|
☀️ Test successful - checks-actions, checks-azure |
Similar to the change in #76669 but all
llvm_asm!is gate in x86/x86_64 target.Godbolt: