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.

Sign up for GitHub

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

Support clobber_abi and vector registers (clobber-only) in PowerPC inline assembly #131341

Merged
merged 1 commit into from
Nov 5, 2024
Merged

Support clobber_abi and vector registers (clobber-only) in PowerPC inline assembly #131341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions compiler/rustc_codegen_gcc/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,8 @@ fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> ConstraintOrRegister {
InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::reg_nonzero) => "b",
InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::freg) => "f",
InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::cr)
| InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::xer) => {
| InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::xer)
| InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::vreg) => {
unreachable!("clobber-only")
}
InlineAsmRegClass::RiscV(RiscVInlineAsmRegClass::reg) => "r",
Expand Down Expand Up @@ -729,7 +730,8 @@ fn dummy_output_type<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, reg: InlineAsmRegCl
InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::reg_nonzero) => cx.type_i32(),
InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::freg) => cx.type_f64(),
InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::cr)
| InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::xer) => {
| InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::xer)
| InlineAsmRegClass::PowerPC(PowerPCInlineAsmRegClass::vreg) => {
unreachable!("clobber-only")
}
InlineAsmRegClass::RiscV(RiscVInlineAsmRegClass::reg) => cx.type_i32(),
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_codegen_llvm/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,9 @@ fn reg_to_llvm(reg: InlineAsmRegOrRegClass, layout: Option<&TyAndLayout<'_>>) ->
PowerPC(PowerPCInlineAsmRegClass::reg) => "r",
PowerPC(PowerPCInlineAsmRegClass::reg_nonzero) => "b",
PowerPC(PowerPCInlineAsmRegClass::freg) => "f",
PowerPC(PowerPCInlineAsmRegClass::cr) | PowerPC(PowerPCInlineAsmRegClass::xer) => {
PowerPC(PowerPCInlineAsmRegClass::cr)
| PowerPC(PowerPCInlineAsmRegClass::xer)
| PowerPC(PowerPCInlineAsmRegClass::vreg) => {
unreachable!("clobber-only")
}
RiscV(RiscVInlineAsmRegClass::reg) => "r",
Expand Down Expand Up @@ -800,7 +802,9 @@ fn dummy_output_type<'ll>(cx: &CodegenCx<'ll, '_>, reg: InlineAsmRegClass) -> &'
PowerPC(PowerPCInlineAsmRegClass::reg) => cx.type_i32(),
PowerPC(PowerPCInlineAsmRegClass::reg_nonzero) => cx.type_i32(),
PowerPC(PowerPCInlineAsmRegClass::freg) => cx.type_f64(),
PowerPC(PowerPCInlineAsmRegClass::cr) | PowerPC(PowerPCInlineAsmRegClass::xer) => {
PowerPC(PowerPCInlineAsmRegClass::cr)
| PowerPC(PowerPCInlineAsmRegClass::xer)
| PowerPC(PowerPCInlineAsmRegClass::vreg) => {
unreachable!("clobber-only")
}
RiscV(RiscVInlineAsmRegClass::reg) => cx.type_i32(),
Expand Down
30 changes: 30 additions & 0 deletions compiler/rustc_target/src/asm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,7 @@ pub enum InlineAsmClobberAbi {
Arm64EC,
RiscV,
LoongArch,
PowerPC,
S390x,
Msp430,
}
Expand Down Expand Up @@ -944,6 +945,10 @@ impl InlineAsmClobberAbi {
"C" | "system" => Ok(InlineAsmClobberAbi::LoongArch),
_ => Err(&["C", "system"]),
},
InlineAsmArch::PowerPC | InlineAsmArch::PowerPC64 => match name {
"C" | "system" => Ok(InlineAsmClobberAbi::PowerPC),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting for myself: I find it slightly amusing that "sysv64" is not valid on platforms which... er... use a System V 64-bit ABI... but it "makes sense" because it's for the AMD64 ABI specifically on Windows-like platforms.

_ => Err(&["C", "system"]),
},
InlineAsmArch::S390x => match name {
"C" | "system" => Ok(InlineAsmClobberAbi::S390x),
_ => Err(&["C", "system"]),
Expand Down Expand Up @@ -1121,6 +1126,31 @@ impl InlineAsmClobberAbi {
f16, f17, f18, f19, f20, f21, f22, f23,
}
},
InlineAsmClobberAbi::PowerPC => clobbered_regs! {
PowerPC PowerPCInlineAsmReg {
// r0, r3-r12
r0,
r3, r4, r5, r6, r7,
r8, r9, r10, r11, r12,

// f0-f13
f0, f1, f2, f3, f4, f5, f6, f7,
f8, f9, f10, f11, f12, f13,

// v0-v19
// FIXME: PPC32 SysV ABI does not mention vector registers processing.
// https://refspecs.linuxfoundation.org/elf/elfspec_ppc.pdf
v0, v1, v2, v3, v4, v5, v6, v7,
v8, v9, v10, v11, v12, v13, v14,
v15, v16, v17, v18, v19,
Comment on lines +1139 to +1145
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I did not treat these as clobbered only on PPC64 is that the PPC32 ABI document I referenced was released at a time when Altivec/VMX did not exist, and I thought it might not reflect the final status of the PPC32 ABI.

In a similar case, the early ABI documents for s390x (e.g., the one mentioned here) do not mention vector registers, but the ABI documents since the addition of vector facility mention them and all are treated as volatile.

Copy link
Member Author

@taiki-e taiki-e Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Power Architecture 32-bit Application Binary Interface Supplement 1.0 - Linux & Embedded published in 2011, PPC32 has the same convention here as PPC64.

Therefore, we can just remove the FIXME comment here.

UPDATE: filed #132638


// cr0-cr1, cr5-cr7, xer
cr0, cr1,
cr5, cr6, cr7,
xer,
// lr and ctr are reserved
}
},
InlineAsmClobberAbi::S390x => clobbered_regs! {
S390x S390xInlineAsmReg {
r0, r1, r2, r3, r4, r5,
Expand Down
78 changes: 75 additions & 3 deletions compiler/rustc_target/src/asm/powerpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
use std::fmt;

use rustc_data_structures::fx::FxIndexSet;
use rustc_span::Symbol;

use super::{InlineAsmArch, InlineAsmType, ModifierInfo};
use crate::spec::{RelocModel, Target};

def_reg_class! {
PowerPC PowerPCInlineAsmRegClass {
reg,
reg_nonzero,
freg,
vreg,
cr,
xer,
}
Expand Down Expand Up @@ -48,11 +51,44 @@ impl PowerPCInlineAsmRegClass {
}
}
Self::freg => types! { _: F32, F64; },
Self::vreg => &[],
Self::cr | Self::xer => &[],
}
}
}

fn reserved_r13(
arch: InlineAsmArch,
_reloc_model: RelocModel,
_target_features: &FxIndexSet<Symbol>,
target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
if target.is_like_aix && arch == InlineAsmArch::PowerPC {
Ok(())
} else {
Err("r13 is a reserved register on this target")
}
}

fn reserved_v20to31(
_arch: InlineAsmArch,
_reloc_model: RelocModel,
_target_features: &FxIndexSet<Symbol>,
target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
if target.is_like_aix {
match &*target.options.abi {
"vec-default" => Err("v20-v31 are reserved on vec-default ABI"),
"vec-extabi" => Ok(()),
_ => unreachable!("unrecognized AIX ABI"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting for myself: I believe this should genuinely not be reachable from user code that isn't using target-spec.json, but I am not 100% confident about this. I should at some point refactor targets so they can emit real errors in these cases, instead. I do not consider this a fact to block this PR over, however, as there is a lot of kinda-questionable handling of some things that might be user-reachable for all the targets, and the worst case here is just an ICE.

}
} else {
Ok(())
}
}

def_regs! {
PowerPC PowerPCInlineAsmReg PowerPCInlineAsmRegClass {
r0: reg = ["r0", "0"],
Expand All @@ -66,6 +102,7 @@ def_regs! {
r10: reg, reg_nonzero = ["r10", "10"],
r11: reg, reg_nonzero = ["r11", "11"],
r12: reg, reg_nonzero = ["r12", "12"],
r13: reg, reg_nonzero = ["r13", "13"] % reserved_r13,
r14: reg, reg_nonzero = ["r14", "14"],
r15: reg, reg_nonzero = ["r15", "15"],
r16: reg, reg_nonzero = ["r16", "16"],
Expand Down Expand Up @@ -113,6 +150,38 @@ def_regs! {
f29: freg = ["f29", "fr29"],
f30: freg = ["f30", "fr30"],
f31: freg = ["f31", "fr31"],
v0: vreg = ["v0"],
v1: vreg = ["v1"],
v2: vreg = ["v2"],
v3: vreg = ["v3"],
v4: vreg = ["v4"],
v5: vreg = ["v5"],
v6: vreg = ["v6"],
v7: vreg = ["v7"],
v8: vreg = ["v8"],
v9: vreg = ["v9"],
v10: vreg = ["v10"],
v11: vreg = ["v11"],
v12: vreg = ["v12"],
v13: vreg = ["v13"],
v14: vreg = ["v14"],
v15: vreg = ["v15"],
v16: vreg = ["v16"],
v17: vreg = ["v17"],
v18: vreg = ["v18"],
v19: vreg = ["v19"],
v20: vreg = ["v20"] % reserved_v20to31,
v21: vreg = ["v21"] % reserved_v20to31,
v22: vreg = ["v22"] % reserved_v20to31,
v23: vreg = ["v23"] % reserved_v20to31,
v24: vreg = ["v24"] % reserved_v20to31,
v25: vreg = ["v25"] % reserved_v20to31,
v26: vreg = ["v26"] % reserved_v20to31,
v27: vreg = ["v27"] % reserved_v20to31,
v28: vreg = ["v28"] % reserved_v20to31,
v29: vreg = ["v29"] % reserved_v20to31,
v30: vreg = ["v30"] % reserved_v20to31,
v31: vreg = ["v31"] % reserved_v20to31,
cr: cr = ["cr"],
cr0: cr = ["cr0"],
cr1: cr = ["cr1"],
Expand All @@ -127,8 +196,6 @@ def_regs! {
"the stack pointer cannot be used as an operand for inline asm",
#error = ["r2", "2"] =>
"r2 is a system reserved register and cannot be used as an operand for inline asm",
#error = ["r13", "13"] =>
"r13 is a system reserved register and cannot be used as an operand for inline asm",
#error = ["r29", "29"] =>
"r29 is used internally by LLVM and cannot be used as an operand for inline asm",
#error = ["r30", "30"] =>
Expand Down Expand Up @@ -163,13 +230,17 @@ impl PowerPCInlineAsmReg {
// Strip off the leading prefix.
do_emit! {
(r0, "0"), (r3, "3"), (r4, "4"), (r5, "5"), (r6, "6"), (r7, "7");
(r8, "8"), (r9, "9"), (r10, "10"), (r11, "11"), (r12, "12"), (r14, "14"), (r15, "15");
(r8, "8"), (r9, "9"), (r10, "10"), (r11, "11"), (r12, "12"), (r13, "13"), (r14, "14"), (r15, "15");
(r16, "16"), (r17, "17"), (r18, "18"), (r19, "19"), (r20, "20"), (r21, "21"), (r22, "22"), (r23, "23");
(r24, "24"), (r25, "25"), (r26, "26"), (r27, "27"), (r28, "28");
(f0, "0"), (f1, "1"), (f2, "2"), (f3, "3"), (f4, "4"), (f5, "5"), (f6, "6"), (f7, "7");
(f8, "8"), (f9, "9"), (f10, "10"), (f11, "11"), (f12, "12"), (f13, "13"), (f14, "14"), (f15, "15");
(f16, "16"), (f17, "17"), (f18, "18"), (f19, "19"), (f20, "20"), (f21, "21"), (f22, "22"), (f23, "23");
(f24, "24"), (f25, "25"), (f26, "26"), (f27, "27"), (f28, "28"), (f29, "29"), (f30, "30"), (f31, "31");
(v0, "0"), (v1, "1"), (v2, "2"), (v3, "3"), (v4, "4"), (v5, "5"), (v6, "6"), (v7, "7");
(v8, "8"), (v9, "9"), (v10, "10"), (v11, "11"), (v12, "12"), (v13, "13"), (v14, "14"), (v15, "15");
(v16, "16"), (v17, "17"), (v18, "18"), (v19, "19"), (v20, "20"), (v21, "21"), (v22, "22"), (v23, "23");
(v24, "24"), (v25, "25"), (v26, "26"), (v27, "27"), (v28, "28"), (v29, "29"), (v30, "30"), (v31, "31");
(cr, "cr");
(cr0, "0"), (cr1, "1"), (cr2, "2"), (cr3, "3"), (cr4, "4"), (cr5, "5"), (cr6, "6"), (cr7, "7");
(xer, "xer");
Expand Down Expand Up @@ -201,5 +272,6 @@ impl PowerPCInlineAsmReg {
reg_conflicts! {
cr : cr0 cr1 cr2 cr3 cr4 cr5 cr6 cr7;
}
// f0-f31 (vsr0-vsr31) and v0-v31 (vsr32-vsr63) do not conflict.
}
}
22 changes: 16 additions & 6 deletions src/doc/unstable-book/src/language-features/asm-experimental-arch.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ This feature tracks `asm!` and `global_asm!` support for the following architect
| NVPTX | `reg32` | None\* | `r` |
| NVPTX | `reg64` | None\* | `l` |
| Hexagon | `reg` | `r[0-28]` | `r` |
| PowerPC | `reg` | `r[0-31]` | `r` |
| PowerPC | `reg_nonzero` | `r[1-31]` | `b` |
| PowerPC | `reg` | `r0`, `r[3-12]`, `r[14-28]` | `r` |
| PowerPC | `reg_nonzero` | `r[3-12]`, `r[14-28]` | `b` |
| PowerPC | `freg` | `f[0-31]` | `f` |
| PowerPC | `vreg` | `v[0-31]` | Only clobbers |
| PowerPC | `cr` | `cr[0-7]`, `cr` | Only clobbers |
| PowerPC | `xer` | `xer` | Only clobbers |
| wasm32 | `local` | None\* | `r` |
Expand Down Expand Up @@ -76,9 +77,10 @@ This feature tracks `asm!` and `global_asm!` support for the following architect
| NVPTX | `reg32` | None | `i8`, `i16`, `i32`, `f32` |
| NVPTX | `reg64` | None | `i8`, `i16`, `i32`, `f32`, `i64`, `f64` |
| Hexagon | `reg` | None | `i8`, `i16`, `i32`, `f32` |
| PowerPC | `reg` | None | `i8`, `i16`, `i32` |
| PowerPC | `reg_nonzero` | None | `i8`, `i16`, `i32` |
| PowerPC | `reg` | None | `i8`, `i16`, `i32`, `i64` (powerpc64 only) |
| PowerPC | `reg_nonzero` | None | `i8`, `i16`, `i32`, `i64` (powerpc64 only) |
| PowerPC | `freg` | None | `f32`, `f64` |
| PowerPC | `vreg` | N/A | Only clobbers |
| PowerPC | `cr` | N/A | Only clobbers |
| PowerPC | `xer` | N/A | Only clobbers |
| wasm32 | `local` | None | `i8` `i16` `i32` `i64` `f32` `f64` |
Expand All @@ -105,6 +107,10 @@ This feature tracks `asm!` and `global_asm!` support for the following architect
| Hexagon | `r29` | `sp` |
| Hexagon | `r30` | `fr` |
| Hexagon | `r31` | `lr` |
| PowerPC | `r1` | `sp` |
| PowerPC | `r31` | `fp` |
| PowerPC | `r[0-31]` | `[0-31]` |
| PowerPC | `f[0-31]` | `fr[0-31]`|
| BPF | `r[0-10]` | `w[0-10]` |
| AVR | `XH` | `r27` |
| AVR | `XL` | `r26` |
Expand Down Expand Up @@ -145,14 +151,18 @@ This feature tracks `asm!` and `global_asm!` support for the following architect
| Architecture | Unsupported register | Reason |
| ------------ | --------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| All | `sp`, `r15` (s390x) | The stack pointer must be restored to its original value at the end of an asm code block. |
| All | `fr` (Hexagon), `$fp` (MIPS), `Y` (AVR), `r4` (MSP430), `a6` (M68k), `r11` (s390x), `x29` (Arm64EC) | The frame pointer cannot be used as an input or output. |
| All | `r19` (Hexagon), `x19` (Arm64EC) | This is used internally by LLVM as a "base pointer" for functions with complex stack frames. |
| All | `fr` (Hexagon), `fp` (PowerPC), `$fp` (MIPS), `Y` (AVR), `r4` (MSP430), `a6` (M68k), `r11` (s390x), `x29` (Arm64EC) | The frame pointer cannot be used as an input or output. |
| All | `r19` (Hexagon), `r29` (PowerPC), `r30` (PowerPC), `x19` (Arm64EC) | These are used internally by LLVM as "base pointer" for functions with complex stack frames. |
| MIPS | `$0` or `$zero` | This is a constant zero register which can't be modified. |
| MIPS | `$1` or `$at` | Reserved for assembler. |
| MIPS | `$26`/`$k0`, `$27`/`$k1` | OS-reserved registers. |
| MIPS | `$28`/`$gp` | Global pointer cannot be used as inputs or outputs. |
| MIPS | `$ra` | Return address cannot be used as inputs or outputs. |
| Hexagon | `lr` | This is the link register which cannot be used as an input or output. |
| PowerPC | `r2`, `r13` | These are system reserved registers. |
| PowerPC | `lr` | The link register cannot be used as an input or output. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think xer should be not supported either and should occupy a line after lr.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, xer is supported as a clobber-only register.

rust/src/doc/unstable-book/src/language-features/asm-experimental-arch.md

Line 39 in d30a53e

| PowerPC | `xer` | `xer` | Only clobbers |

See https://rust-lang.zulipchat.com/#narrow/stream/216763-project-inline-asm/topic/preserve_flags.20on.20OpenPower.2FPowerPC.3F/near/251292056 for more context.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I thought these lines are for input or output operands. It makes sense xer as clobber operand since there are instructions changing the ca bit of xer.

| PowerPC | `ctr` | The counter register cannot be used as an input or output. |
| PowerPC | `vrsave` | The vrsave register cannot be used as an input or output. |
Comment on lines +162 to +165
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a UI test for these? akin to https://github.com/rust-lang/rust/blob/4d88de2acd2cf8595c61e5c8d39183e5579204d8/tests/ui/asm/x86_64/bad-reg.rs

Copy link
Member Author

@taiki-e taiki-e Nov 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ui test for this: https://github.com/rust-lang/rust/pull/131341/files#diff-ad23a8c300c887c5ca754ccb89f97d9a7c5cf714df4ba260adb0055b6678f456

Btw, such unsupported registers are present in most architectures, but only aarch64/arm64ec, x86_64, and not yet merged sparc/sparc64 (and powerpc/powerpc64 by this PR) currently have ui tests for them. I plan to add tests for other arches later.

| AVR | `r0`, `r1`, `r1r0` | Due to an issue in LLVM, the `r0` and `r1` registers cannot be used as inputs or outputs. If modified, they must be restored to their original values before the end of the block. |
|MSP430 | `r0`, `r2`, `r3` | These are the program counter, status register, and constant generator respectively. Neither the status register nor constant generator can be written to. |
| M68k | `a4`, `a5` | Used internally by LLVM for the base pointer and global base pointer. |
Expand Down
Loading
Loading