Add inline asm support for amdgpu #149793
Conversation
This comment has been minimized.
This comment has been minimized.
|
Some changes occurred in compiler/rustc_codegen_gcc |
|
This seems okay to me, but I'd rather someone more familiar with this part of the compiler give the final signoff. @bors r? |
|
Error: Parsing assign command in comment failed: ...'' | error: specify user to assign to at >| ''... Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip. |
|
@bors r? compiler |
|
@rustbot reroll |
bdb726b to
9db5dca
Compare
|
Removed return type from tests to fix conflict with #149991, which starts disallowing returns in |
|
The change seems Ok, i'd like people with more background to take a look. |
|
That's not me (sorry it took me a while because of holidays). But iirc that could be amanieu? r? @Amanieu |
|
☔ The latest upstream changes (presumably #150866) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@taiki-e, I saw your mention in the asm tracking issue. amdgpu has named registers (>384 of them), so a clobber_abi would be possible. However, there’s currently no stable ABI, so we would need to follow what the LLVM backend does and I think there’s no tools to make sure the Rust and LLVM ABI understanding is in sync, so that would be asking for subtle breakages. Therefore I think it doesn’t make sense to support clobber_abi with amdgpu at the moment. |
|
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. |
|
Rebased to fix conflict, no other changes Edit: kdiff3 didn’t like 🦀 when fixing conflicts 😢, fixed now |
This comment has been minimized.
This comment has been minimized.
Add support for inline assembly for the amdgpu backend (the amdgcn-amd-amdhsa target). Add register classes for `vgpr` (vector general purpose register) and `sgpr` (scalar general purpose register). The LLVM backend supports two more classes, `reg`, which is either VGPR or SGPR, up to the compiler to decide. As instructions often rely on a register being either a VGPR or SGPR for the assembly to be valid, reg doesn’t seem that useful (I struggled to write correct tests for it), so I didn’t end up adding it. The fourth register class is AGPRs, which only exist on some hardware versions (not the consumer ones) and they have restricted ways to write and read from them, which makes it hard to write a Rust variable into them. They could be used inside assembly blocks, but I didn’t add them as Rust register class. There is one change affecting general inline assembly code, that is `InlineAsmReg::name()` now returns a `Cow` instead of a `&'static str`. Because amdgpu has many registers, 256 VGPRs plus combinations of 2 or 4 VGPRs, and I didn’t want to list hundreds of static strings, the amdgpu reg stores the register number(s) and a non-static String is generated at runtime for the register name.
Add support for inline assembly for the amdgpu backend (the amdgcn-amd-amdhsa target).
Add register classes for
vgpr(vector general purpose register) andsgpr(scalar general purpose register).The LLVM backend supports two more classes,
reg, which is either VGPR or SGPR, up to the compiler to decide. As instructions often rely on a register being either a VGPR or SGPR for the assembly to be valid, reg doesn’t seem that useful (I struggled to write correct tests for it), so I didn’t end up adding it.The fourth register class is AGPRs, which only exist on some hardware versions (not the consumer ones) and they have restricted ways to write and read from them, which makes it hard to write a Rust variable into them. They could be used inside assembly blocks, but I didn’t add them as Rust register class.
There is one change affecting general inline assembly code, that is
InlineAsmReg::name()now returns aCowinstead of a&'static str. Because amdgpu has many registers, 256 VGPRs plus combinations of 2 or 4 VGPRs, and I didn’t want to list hundreds of static strings, the amdgpu reg stores the register number(s) and a non-static String is generated at runtime for the register name.Tracking issue: #135024