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

asm! needs a better way of specifying clobbers #81092

Closed
Amanieu opened this issue Jan 16, 2021 · 13 comments · Fixed by #87581
Closed

asm! needs a better way of specifying clobbers #81092

Amanieu opened this issue Jan 16, 2021 · 13 comments · Fixed by #87581
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) C-enhancement Category: An issue proposing an enhancement or a PR with one. F-asm `#![feature(asm)]` (not `llvm_asm`) T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Amanieu
Copy link
Member

Amanieu commented Jan 16, 2021

Clobbered registers are currently represented as an output to _. However there are several limitations to this approach:

  • It is impossible for registers that are not supported as inputs/outputs (e.g. x87 float registers, MMX registers, RISC-V vector registers, etc) to be marked as clobbered.
  • The set of registers available for clobbering is limited by the currently enabled target features. For example, it is desirable to mark SSE registers as clobbered even if SSE instructions are disabled (e.g. for a kernel). Having to wrap everything in #[cfg(target_feature = "sse")] is very inconvenient. Fixed by Allow clobbering unsupported registers in asm! #83841
  • Lack of forwards compatibility with new registers that could be introduced with future ISA extensions. Consider the AVX-512 mark registers (k0-k7): if an asm! block calls a Rust function, that function may write to the mask registers. However the mask registers may not have been clobbered if the author of the asm! was not aware of AVX-512 when it was written.

Some potential solutions include:

  • Adding a separate clobber("reg") operand which is similar to the existing ones but doesn't require that the target feature for the register be enabled. Fixed by Allow clobbering unsupported registers in asm! #83841
  • Adding some sort of wildcard clobber. For example clobber("all") would clobber all registers known to the compiler. This would be incompatible with register class specifiers: you would need to explicitly specify registers for all input/output operands.
  • Adding ABI-based clobbers such as clobber("C") which clobbers all registers that would be clobbered by a call to an extern "C" fn.

It is not yet clear what the best solution is here. Discussion is welcome.

@Amanieu Amanieu added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-inline-assembly Area: Inline assembly (`asm!(…)`) F-asm `#![feature(asm)]` (not `llvm_asm`) labels Jan 16, 2021
@jonas-schievink jonas-schievink added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 16, 2021
@bkolobara
Copy link

The clobber("all") option would be perfect for our use case. Currently, a big part of our asm! macro is just a list of all registers with different combinations for different architectures. It's hard to maintain, especially as you mentioned the forwards compatibility could be an issue later.

@joshtriplett
Copy link
Member

All three of these approaches seem useful. We may or may not need all of them before we can stabilize asm, though.

clobber("C") seems like a valuable one to have, to safely call C functions without having to manually maintain a perpetually incomplete clobber list.

@bkolobara Do you specifically need clobber("all"), or would clobber("C") suffice for your use case? The latter could likely be more usable, in that you wouldn't need to specify explicit registers for each input or output.

Could we separate this into a few orthogonal items, and determine which ones block stabilization of asm!?

@Amanieu
Copy link
Member Author

Amanieu commented Feb 3, 2021

There's also some considerations from LLVM to take into account based on how it handles clobbers:

  • Unlike inputs/outputs, LLVM clobbers don't depend on active target features (invalid clobbers are just ignored by the compiler). For example you can clobber SSE registers without the SSE feature enabled.
  • Clobbers conflict with both inputs and outputs. So the same register can't be used as both an intput/output and a clobber. If we clobber all registers then there are no registers left for LLVM to allocate for outputs.
  • Because of the above, lateout("reg") _ can't be converted to an LLVM clobber, only out("reg") _ can.
  • If we introduce clobber("reg") syntax, should we continue supporting out("reg") _ as well?

@bkolobara
Copy link

@joshtriplett We specifically need clobber("all"), because we are using it for a generator implementation and after the asm! call we can't guarantee anything.

@jrvanwhy
Copy link
Contributor

  • It is impossible for registers that are not supported as inputs/outputs (e.g. x87 float registers, MMX registers, RISC-V vector registers, etc) to be marked as clobbered.

Is r14 (aka lr) on ARM one of these registers? When I try to mark r14 clobbered I get the following error:

error: couldn't allocate output register for constraint '{r14}'

@Amanieu
Copy link
Member Author

Amanieu commented Feb 13, 2021

That's a bug, rustc should be passing {lr} instead of {r14} to LLVM in the asm constraint string.

If you want to submit a PR, the fix is fairly straightforward: you just need to do the same thing that is done for AArch64's x30 here:

} else if reg == InlineAsmReg::AArch64(AArch64InlineAsmReg::x30) {
// LLVM doesn't recognize x30
"{lr}".to_string()
} else {

@jrvanwhy
Copy link
Contributor

That's a bug, rustc should be passing {lr} instead of {r14} to LLVM in the asm constraint string.

Thank you! It sounds like my problem is separate from this issue, so I opened a #82052.

@hudson-ayers
Copy link
Contributor

On a similar note, is r6 on ARM one of these registers? Currently, attempting to mark it as clobbered produces this error:

error: invalid register `r6`: r6 is used internally by LLVM and cannot be used as an operand for inline asm
   --> arch/cortex-m/src/lib.rs:254:31
    |
254 |     out("r4") _, out("r5") _, out("r6") _, out("r8") _, out("r9") _,
    |                               ^^^^^^^^^^^

A similar error is produced if you attempt to mark r7 as clobbered, because r7 is used as the frame pointer.

@Amanieu
Copy link
Member Author

Amanieu commented Feb 23, 2021

No, that's intentional. LLVM will sometimes use r6 internally for stack-relative addressing.

@hudson-ayers
Copy link
Contributor

Does that mean it is invalid for inline assembly to trigger a context switch to a userspace that will modify r6 and eventually return within the same assembly block? Or does it mean that the programmer is responsible for saving and restoring r6 before / after the calls that will modify it?

I have struggled to find any reference about when LLVM will modify/use r6. Sorry, I realize this may not be the appropriate place for this question.

@Amanieu
Copy link
Member Author

Amanieu commented Feb 23, 2021

It means that the programmer is responsible for saving and restoring r6. The references are deep in LLVM's source code and not documented anywhere. However we have to do this since LLVM may generate incorrect code otherwise (by not saving/restoring r6 properly).

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 4, 2021
Allow clobbering unsupported registers in asm!

Previously registers could only be marked as clobbered if the target feature for that register was enabled. This restriction is now removed.

cc rust-lang#81092

r? `@nagisa`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 4, 2021
Allow clobbering unsupported registers in asm!

Previously registers could only be marked as clobbered if the target feature for that register was enabled. This restriction is now removed.

cc rust-lang#81092

r? ``@nagisa``
@Amanieu
Copy link
Member Author

Amanieu commented Apr 13, 2021

One significant issue that was discussed in Zulip is how wildcard clobbers should handle registers that are reserved by LLVM.

@Amanieu
Copy link
Member Author

Amanieu commented Apr 14, 2021

I think the best way forward is to add a clobber_abi("$ABI") argument to asm! where $ABI can be "C", "stdcall", "sys64", etc. This has the following effect:

  • Registers that are not explicitly specified as operands are allowed to be clobbered if they are caller-saved registers according to the chosen calling convention.
  • This also applies to registers that are normally disallowed as asm! operands, e.g. x87 registers.
  • LLVM reserved registers are always callee-saved, so we don't need to worry about them.
    • This is not actually true on all targets (e.g. PowerPC reserves LR which is caller-saved). Those targets will need to be fixed or worked around before we enable clobber_abi for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) C-enhancement Category: An issue proposing an enhancement or a PR with one. F-asm `#![feature(asm)]` (not `llvm_asm`) T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants