Support clobber_abi in AVR inline assembly
#131323
Open
+66
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds the relevant registers to the list of clobbered regis- ters (part of rust-lang#93335). This follows the [ABI documentation] of AVR-GCC: > The [...] call-clobbered general purpose registers (GPRs) are > registers that might be destroyed (clobbered) by a function call. > > - **R18–R27, R30, R31** > > These GPRs are call clobbered. An ordinary function may use them > without restoring the contents. [...] > > - **R0, T-Flag** > > The temporary register and the T-flag in SREG are also call- > clobbered, but this knowledge is not exposed explicitly to the > compiler (R0 is a fixed register). Therefore this commit lists the aforementioned registers `r18–r27`, `r30` and `r31` as clobbered registers. Since the `r0` register (listed above as well) is not available in inline assembly at all (potentially because the AVR-GCC considers it a fixed register causing the register to never be used in register allocation and LLVM adopting this), there is no need to list it in the clobber list (the `r0`-variant is not even available). A comment was added to ensure, that the `r0` gets added to the clobber-list once the register gets usable in inline ASM. Since the SREG is normally considered clobbered anyways (unless the user supplies the `preserve_flags`-option), there is no need to explicitly list a bit in this register (which is not possible to list anyways). Note, that this commit completely ignores the case of interrupts (that are described in the ABI-specification), since every register touched in an ISR need to be saved anyways. [ABI documentation]: https://gcc.gnu.org/wiki/avr-gcc#Call-Used_Registers
rustbot
added
S-waiting-on-review
jfrimmel
force-pushed
the
avr-inline-asm-clobber-abi
branch
from
e6e6b84
to
5d58879
Compare
Amanieu
reviewed
Oct 11, 2024
| @@ -106,6 +106,7 @@ def_regs! { | |||
| "the stack pointer cannot be used as an operand for inline asm", | |||
| #error = ["r0", "r1", "r1r0"] => | |||
| "r0 and r1 are not available due to an issue in LLVM", | |||
| // if this issue is resolved, then R0 has to be added to clobbered registers | |||
There was a problem hiding this comment.
We need to carefully keep track of LLVM here: if LLVM ceases to treat these registers as reserved then we must immediately add them to the set of clobbered registers, otherwise incorrect code will be generated. This comment should be updated to reflect this.
Also, while reviewing the GCC docs, it seems that R1 always holds a value of 0. If this is a guarantee that asm can use then it should be documented, similar to how on x86 we guarantee the direction flag is always clear when entering or exiting inline asm.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the
clobber_abipart necessary to eventually stabilize the inline assembly for AVR. This is tracked in #93335.This is heavily inspired by the sibling-PR #131310 for the MSP430. I've explained my reasoning in the first commit message in detail, which is reproduced below for easier reviewing:
This follows the ABI documentation of AVR-GCC:
Therefore this commit lists the aforementioned registers
r18–r27,r30andr31as clobbered registers. Since ther0register (listed above as well) is not available in inline assembly at all (potentially because the AVR-GCC considers it a fixed register causing the register to never be used in register allocation and LLVM adopting this), there is no need to list it in the clobber list (ther0-variant is not even available). A comment was added to ensure, that ther0gets added to the clobber-list once the register gets usable in inline ASM.Since the SREG is normally considered clobbered anyways (unless the user supplies the
preserve_flags-option), there is no need to explicitly list a bit in this register (which is not possible to list anyways).Note, that this commit completely ignores the case of interrupts (that are described in the ABI-specification), since every register touched in an ISR need to be saved anyways.
r? @Amanieu
@rustbot label +O-AVR