asm! support for the Xtensa architecture #147302
Open
+356
−2
Conversation
This file contains hidden or 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
Collaborator
|
Some changes occurred in compiler/rustc_codegen_gcc |
rustbot
added
A-LLVM
rustbot
added
the
T-compiler
This comment has been minimized.
This comment has been minimized.
11 tasks
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bjorn3
reviewed
Oct 3, 2025
Amanieu
reviewed
Oct 3, 2025
| def_reg_class! { | ||
| Xtensa XtensaInlineAsmRegClass { | ||
| reg, | ||
| freg, |
Member
There was a problem hiding this comment.
LLVM doesn't seem to support this. XtensaTargetLowering::getRegForInlineAsmConstraint only handles r constraints.
Contributor
Author
There was a problem hiding this comment.
It will just not yet. I'll back out the floating point bits for now, unless you think it's okay to land ready for the LLVM updates.
Collaborator
|
☔ The latest upstream changes (presumably #147645) made this pull request unmergeable. Please resolve the merge conflicts. |
This comment has been minimized.
This comment has been minimized.
Collaborator
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Taiki Endo <[email protected]> Co-authored-by: Kerry Jones <[email protected]>
Contributor
Author
|
I've made most of the updates here (sorry for the delay!). It turns out we'll need a patch that landed just after rust branched LLVM: https://github.com/rust-lang/llvm-project/tree/rustc/21.1-2025-08-01/llvm/lib/Target/Xtensa , otherwise the asm tests fail :(. The good news is that we've since landed FP support in LLVM upstream too, so maybe the next time rust branches LLVM 21 (I guess 21.2?) we can rerun CI on this PR and it will be green. |
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Amanieu
reviewed
Nov 23, 2025
Comment on lines
+69
to
+71
| // The Xtensa arch doesn't require, nor use, a dedicated frame pointer register | ||
| // therefore if it is not force enabled, we can assume it won't be generated. | ||
| // If frame pointers are enabled, we cannot use the register as a general purpose one. |
Member
There was a problem hiding this comment.
Unfortunately that's not exactly true. LLVM will force the use of a frame pointer if the stack frame has variable-sized objects. While this currently isn't possible in Rust, it is possible when doing cross-language LTO with C code. Therefore we should always reject the use of the frame pointer in asm.
| a4: reg = ["a4"], | ||
| a5: reg = ["a5"], | ||
| a6: reg = ["a6"], | ||
| a7: reg = ["a7"], |
Member
There was a problem hiding this comment.
Looking at the LLVM code, it seems use a7 instead of a15 as the frame pointer when the windowed ABI is used.
| a13: reg = ["a13"], | ||
| a14: reg = ["a14"], | ||
| a15: reg = ["a15"] % is_frame_pointer, | ||
| sar: reg = ["sar"], |
Member
There was a problem hiding this comment.
This shouldn't be part of the reg class since it doesn't usually make sense for asm to select this regiser for the reg constraint. It should probably be in its own clobber-only register class.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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 implements the asm! support for Xtensa. We've been using this code for a few years in our fork and it's been working well. I finally found some time to clean it up a bit and start the upstreaming process. This should be one of the final PRs for Xtensa support on the Rust side (minus bug fixes of course). After this, we're mostly just waiting on the LLVM upstreaming which is going well. This PR doesn't cover all possible asm options for Xtensa, but the base ISA plus a few extras that are used in Espressif chips.
r? Amanieu