Add support for clobber_abi to asm!#87581
Conversation
rust-highfive
added
the
S-waiting-on-review
nagisa
left a comment
nagisa
left a comment
There was a problem hiding this comment.
Seems reasonable enough, though some integration with existing abi machinery in rustc would be ideal. For example I could see adjust_cabi being changed so that is converts C to win64 or sysv64 or vice versa on the relevant systems.
compiler/rustc_target/src/asm/mod.rs
Outdated
There was a problem hiding this comment.
I think it is somewhat important that we utilize Target::adjusted_abi here so that e.g. ABIs such as system and efiapi work out of the box here.
There was a problem hiding this comment.
I tried using Abi directly but the code got a bit messy. In any case we only support a small subset of ABIs for clobber_abi, I'm not sure if it makes sense to use the existing ABI machinery.
compiler/rustc_target/src/asm/mod.rs
Outdated
There was a problem hiding this comment.
Why do we handle aapcs for ARM but not aarch64?
There was a problem hiding this comment.
On AArch64
pub extern "aapcs" fn aapcs_func() {}results in:
LLVM ERROR: Unsupported calling convention.
There was a problem hiding this comment.
aapcs is only a valid calling convention on ARM, not AArch64.
compiler/rustc_target/src/asm/mod.rs
Outdated
There was a problem hiding this comment.
Can you handle the "system" abi on all archs too? This matches whatever abi is used on the respective platform to call into system libraries, so the same as the "C" abi on most targets, but on 32bit x86 windows it is the stdcall calling convention.
There was a problem hiding this comment.
Sure, I can add "system" (and even "efiapi") to this list. It makes no difference in terms of which registers are actually clobbered since cdecl and stdcall clobber the same set of registers, and on other arches they are identical to "C".
This comment has been minimized.
This comment has been minimized.
Amanieu
force-pushed
the
asm_clobber_abi
branch
from
66822a7 to
fe1d6fb
Compare
This comment has been minimized.
This comment has been minimized.
Amanieu
force-pushed
the
asm_clobber_abi
branch
from
fe1d6fb to
d706455
Compare
This comment has been minimized.
This comment has been minimized.
Amanieu
force-pushed
the
asm_clobber_abi
branch
from
d706455 to
0726565
Compare
This comment has been minimized.
This comment has been minimized.
Amanieu
force-pushed
the
asm_clobber_abi
branch
from
6f3a92e to
3fd463a
Compare
|
@bors r+ |
|
📌 Commit 3fd463a has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
7 participants
This PR adds the
clobber_abifeature that was proposed in #81092.Fixes #81092
cc @rust-lang/wg-inline-asm
r? @nagisa