Add support for clobber_abi to asm! #87581
Conversation
rust-highfive
added
the
S-waiting-on-review
| arch: InlineAsmArch, | ||
| target: &Target, | ||
| name: Symbol, | ||
| ) -> Result<Self, &'static [&'static str]> { |
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
| _ => Err(&["C", "aapcs"]), | ||
| }, | ||
| InlineAsmArch::AArch64 => match name { | ||
| "C" => Ok(InlineAsmClobberAbi::AArch64), |
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
| let name = &*name.as_str(); | ||
| match arch { | ||
| InlineAsmArch::X86 => match name { | ||
| "C" | "cdecl" | "stdcall" | "fastcall" => Ok(InlineAsmClobberAbi::X86), |
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 |
This PR adds the
clobber_abifeature that was proposed in #81092.Fixes #81092
cc @rust-lang/wg-inline-asm
r? @nagisa