-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add asm!() support for hexagon #73214
Conversation
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
r? @Amanieu This is probably a good start for inline asm w/Hexagon. Missing overlapping regs implementation and more tests. Not many of the registers defined so far actually overlap, just p3_0 and p3/p2/p1/p0 I think. Haven't quite deciphered the logic in the other targets' macro-based implementation, but I'll figure it out. |
#[no_mangle] | ||
pub unsafe fn gpr_transfer_sym() { | ||
// Hack to avoid function merging | ||
extern "Rust" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dont_merge
might not be necessary here. I just threw this in because it seemed like the functions were reordered and I didn't know if function merging was responsible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function reordering can't be the issue, CHECK-LABEL
should handle that for you automatically.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
0e0ce04
to
fd5f2f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HVX test is failing and predicate registers aren't implemented/tested yet.
Incorporated some review feedback.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
fd5f2f4
to
f2b482f
Compare
f2b482f
to
a217ce1
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
9ce351f
to
1d23724
Compare
I think this failure is CI infrastructure related. Can I just retry it?
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
type ptr_8 = *const i8; | ||
type ptr_16 = *const i16; | ||
type ptr_32 = *const i32; | ||
type ptr_mut_32 = *mut i32; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only need to test one pointer type here, they are all treated identically by the backend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ptr_mut_32
is not used anywhere. I would also recommend renaming ptr_32
to just ptr
for consistency with the other asm tests.
You can find the set of constraint codes and types supported by LLVM here. |
Yeah, sorry, we just don't have support for these all upstreamed yet. I'll get on it. |
1d23724
to
6b774c8
Compare
Thanks for your guidance @Amanieu ! Hopefully we can consider adding just support for general purpose regs for now and I can follow up with changes to support pairs. And after I push up some backend stuff I can add vector regs and more. |
cc @kparzysz-quic and @SidManning in case you want to look over this code change that adds hexagon inline asm register support to rust. |
@@ -623,6 +626,7 @@ The supported modifiers are a subset of LLVM's (and GCC's) [asm template argumen | |||
| NVPTX | `reg64` | None | `rd0` | None | | |||
| RISC-V | `reg` | None | `x1` | None | | |||
| RISC-V | `freg` | None | `f0` | None | | |||
| Hexagon | `reg` | None | `r0` | `r` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LLVM modifier is actually None
. This is not the same thing as the constraint code.
@@ -578,6 +580,7 @@ Some registers cannot be used for input or output operands: | |||
| ARM | `pc` | This is the program counter, not a real register. | | |||
| RISC-V | `x0` | This is a constant zero register which can't be modified. | | |||
| RISC-V | `gp`, `tp` | These registers are reserved and cannot be used as inputs or outputs. | | |||
| Hexagon | `lr` | This is the link register which cannot be used as an input or output. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add sp
, lr
and fr
as aliases for x29
, x30
and x31
in the register names tables.
@@ -507,6 +508,7 @@ Each register class has constraints on which value types they can be used with. | |||
| RISC-V64 | `reg` | None | `i8`, `i16`, `i32`, `f32`, `i64`, `f64` | | |||
| RISC-V | `freg` | `f` | `f32` | | |||
| RISC-V | `freg` | `d` | `f64` | | |||
| Hexagon | `reg` | `r` | `i8`, `i16`, `i32`, `f32` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The target feature should be None
: general-purpose registers are always available.
3b90ddd
to
7a9f29d
Compare
@bors r+ |
📌 Commit 7a9f29d has been approved by |
|
||
Some registers cannot be used for input or output operands: | ||
|
||
| Architecture | Unsupported register | Reason | | ||
| ------------ | -------------------- | ------ | | ||
| All | `sp` | The stack pointer must be restored to its original value at the end of an asm code block. | | ||
| All | `bp` (x86), `r11` (ARM), `x29` (AArch64), `x8` (RISC-V) | The frame pointer cannot be used as an input or output. | | ||
| All | `bp` (x86), `r11` (ARM), `x29` (AArch64), `x8` (RISC-V), `fr` (Hexagon) | The frame pointer cannot be used as an input or output. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All registers can be used as inputs on Hexagon, that is the architecture doesn't not differentiate between r0
and r29
or r30
, except for a small set of instructions which require fewer bits to encode registers. As a matter of fact, most loads/stores to stack are done via r29
or r30
(i.e. sp
or fp
), and a return from a function that does not allocate stack space is done via jumpr r31
.
All of these registers can also be modified, but that's typically a bad idea.
Add asm!() support for hexagon
Add asm!() support for hexagon
…arth Rollup of 13 pull requests Successful merges: - rust-lang#71568 (Document unsafety in slice/sort.rs) - rust-lang#72709 (`#[deny(unsafe_op_in_unsafe_fn)]` in liballoc) - rust-lang#73214 (Add asm!() support for hexagon) - rust-lang#73248 (save_analysis: improve handling of enum struct variant) - rust-lang#73257 (ty: projections in `transparent_newtype_field`) - rust-lang#73261 (Suggest `?Sized` when applicable for ADTs) - rust-lang#73300 (Implement crate-level-only lints checking.) - rust-lang#73334 (Note numeric literals that can never fit in an expected type) - rust-lang#73357 (Use `LocalDefId` for import IDs in trait map) - rust-lang#73364 (asm: Allow multiple template string arguments; interpret them as newline-separated) - rust-lang#73382 (Only display other method receiver candidates if they actually apply) - rust-lang#73465 (Add specialization of `ToString for char`) - rust-lang#73489 (Refactor hir::Place) Failed merges: r? @ghost
Add asm!() support for PowerPC This includes GPRs and FPRs only. Note that this does not include PowerPC64. For my reference, this was mostly duplicated from PR rust-lang#73214.
No description provided.