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.

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

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Oct 7, 2024

This supports clobber_abi which is one of the requirements of stabilization mentioned in #93335.

This basically does a similar thing I did in #130630 to implement clobber_abi for s390x, but for powerpc/powerpc64/powerpc64le.

  • This also supports vector registers (as vreg) as clobber-only, which need to support clobbering of them to implement clobber_abi.
  • vreg should be able to accept #[repr(simd)] types as input/output if the unstable altivec target feature is enabled, but core::arch::{powerpc,powerpc64} vector types, #[repr(simd)], and core::simd are all unstable, so the fact that this is currently a clobber-only should not be considered a blocker of clobber_abi implementation or stabilization. So I have not implemented it in this PR.

Refs:

If I understand the above four ABI documentations correctly, except for the PPC32 SysV's VR (Vector Registers) and 32-bit AIX (currently not supported by rustc)'s r13, there does not appear to be important differences in terms of implementing clobber_abi:

  • The above four ABIs are consistent about FPR (0-13: volatile, 14-31: nonvolatile), CR (0-1,5-7: volatile, 2-4: nonvolatile), XER (volatile), and CTR (volatile).

  • As for GPR, only the registers we are treating as reserved are slightly different

    • r0, r3-r12 are volatile
    • r1(sp, reserved), r14-31 are nonvolatile
    • r2(reserved) is TOC pointer in PPC64 ELF/AIX, system-reserved register in PPC32 SysV (AFAIK used as thread pointer in Linux/BSDs)
    • r13(reserved for non-32-bit-AIX) is thread pointer in PPC64 ELF, small data area pointer register in PPC32 SysV, "reserved under 64-bit environment; not restored across system calls1" in AIX)
  • As for FPSCR, volatile in PPC64 ELFv1/AIX, some fields are volatile only in certain situations (rest are volatile) in PPC32 SysV/PPC64 ELFv2.

  • As for VR (Vector Registers), it is not mentioned in PPC32 SysV, v0-v19 are volatile in both in PPC64 ELF/AIX, v20-v31 are nonvolatile in PPC64 ELF, reserved or nonvolatile depending on the ABI (vec-extabi vs vec-default in LLVM, we are using vec-extabi) in AIX:

    When the default Vector enabled mode is used, these registers are reserved and must not be used.
    In the extended ABI vector enabled mode, these registers are nonvolatile and their values are preserved across function calls

    I left FIXME comment about PPC32 SysV and added ABI check for AIX.

  • As for VRSAVE, it is not mentioned in PPC32 SysV, nonvolatile in PPC64 ELFv1, reserved in PPC64 ELFv2/AIX

  • As for VSCR, it is not mentioned in PPC32 SysV/PPC64 ELFv1, some fields are volatile only in certain situations (rest are volatile) in PPC64 ELFv2, volatile in AIX

We are currently treating r1-r2, r13 (non-32-bit-AIX), r29-r31, LR, CTR, and VRSAVE as reserved.
We are currently not processing anything about FPSCR and VSCR, but I feel those are things that should be processed by preserves_flags rather than clobber_abi if we need to do something about them. (However, PPCRegisterInfo.td in LLVM does not seem to define anything about them.)

Replaces #111335 and #124279

cc @ecnelises @bzEq @lu-zero

r? @Amanieu

@rustbot label +O-PowerPC +A-inline-assembly

Footnotes

  1. callee-saved, according to LLVM and GCC.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 7, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 7, 2024

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rustbot rustbot added the O-PowerPC Target: PowerPC processors label Oct 7, 2024
@rust-log-analyzer

This comment has been minimized.

@taiki-e taiki-e force-pushed the ppc-clobber-abi branch 2 times, most recently from 76500b8 to fc3dc84 Compare October 7, 2024 15:45
Comment on lines +1121 to +1145

// 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,
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

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

Comment on lines 138 to 139
// FIXME: In AIX, v20-v31 are reserved or nonvolatile depending on the mode.
// https://www.ibm.com/docs/en/aix/7.3?topic=concepts-aix-vector-programming
Copy link
Member Author

Choose a reason for hiding this comment

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

@ecnelises @bzEq: Is the default in Rust the default Vector enabled mode, as the name implies? Also, is there a way provided for the compiler to understand the current mode?

(If the first is yes and the second is no, it would be sufficient to simply reject the use of v20-v31 as reserved. If the first is no, this code is fine as is.)

Copy link
Contributor

Choose a reason for hiding this comment

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

It should refer to vec-extabi, which is the default. See

abi: "vec-extabi".into(),
.

@taiki-e
Copy link
Member Author

taiki-e commented Oct 11, 2024

  • vreg should be able to accept #[repr(simd)] types as input/output if the unstable altivec target feature is enabled, but core::arch::{powerpc,powerpc64} vector types, #[repr(simd)], and core::simd are all unstable, so the fact that this is currently a clobber-only should not be considered a blocker of clobber_abi implementation or stabilization. So I have not implemented it in this PR.

Opened #131551 (which is based on this PR) to implement this.

(I'm not sticking to whether that PR should be a separate PR or part of this PR, so I can merge that PR into this PR if needed.)

@taiki-e taiki-e force-pushed the ppc-clobber-abi branch 2 times, most recently from 9fbedcb to bad0db8 Compare October 12, 2024 12:10
@bzEq
Copy link
Contributor

bzEq commented Oct 12, 2024

r13 is thread pointer in PPC64 ELF, ..., "reserved under 64-bit environment" in AIX

In fact, r13 is also thread pointer in AIX-64 pthread environment, can be considered reserved.

@taiki-e taiki-e force-pushed the ppc-clobber-abi branch 2 times, most recently from 7eee8ce to 369edf1 Compare October 12, 2024 14:16
@taiki-e
Copy link
Member Author

taiki-e commented Oct 12, 2024

Updated to check ABI in v20-v31 for AIX, and update def_regs to reflect the fact that r13 is not reserved on 32-bit AIX (see also LLVM's getReservedRegs). (32-bit AIX is currently not supported by rustc, although)

@taiki-e taiki-e force-pushed the ppc-clobber-abi branch 2 times, most recently from f3a4182 to d30a53e Compare October 12, 2024 15:55
| All | `r19` (Hexagon), `x19` (Arm64EC) | This is used internally by LLVM as a "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 | `$r29`, `$r30` | These are used internally by LLVM. |
Copy link
Contributor

Choose a reason for hiding this comment

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

It's ok to not support these two registers conservasively IMO, might rephrase to These might be used as base pointer inside LLVM.

Copy link
Member Author

@taiki-e taiki-e Oct 13, 2024

Choose a reason for hiding this comment

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

I found a line that collectively describes registers in the same situation, so I moved these two there.

| All | `r19` (Hexagon), `r29` (PowerPC), `r30` (PowerPC), `x19` (Arm64EC) | These are used internally by LLVM as "base pointer" for functions with complex stack frames. |

| All | `r19` (Hexagon), `x19` (Arm64EC) | This is used internally by LLVM as a "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. |
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Separate r2 and r13.
r2 is used as TOC pointer.
r13 is system reserved register.

Copy link
Member Author

Choose a reason for hiding this comment

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

Those explanations were just copied from the error message, but AFAIK r2 is used as a thread pointer in the PPC32 SVR4 ABI (Linux, FreeBSD, NetBSD, OpenBSD), so I think the current explanation is actually fine as is.

#error = ["r2", "2"] =>
"r2 is a system reserved register and cannot be used as an operand for inline asm",

| All | `r19` (Hexagon), `x19` (Arm64EC) | This is used internally by LLVM as a "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 | `$r29`, `$r30` | These are used internally by LLVM. |
| 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.

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.

@taiki-e taiki-e force-pushed the ppc-clobber-abi branch 4 times, most recently from 0633ee4 to cb969ca Compare October 13, 2024 16:27
@bzEq
Copy link
Contributor

bzEq commented Oct 13, 2024

I have no more comments and LGTM.

@rustbot rustbot added the A-inline-assembly Area: Inline assembly (`asm!(…)`) label Oct 13, 2024
@bors
Copy link
Contributor

bors commented Oct 22, 2024

@bzEq: 🔑 Insufficient privileges: Not in reviewers

@bors
Copy link
Contributor

bors commented Oct 22, 2024

@bzEq: 🔑 Insufficient privileges: not in try users

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.

@@ -943,6 +944,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.

Comment on lines +162 to +165
| PowerPC | `r2`, `r13` | These are system reserved registers. |
| PowerPC | `lr` | The link register cannot be used as an input or output. |
| 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. |
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

@taiki-e taiki-e Nov 2, 2024

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.

@bors
Copy link
Contributor

bors commented Nov 2, 2024

☔ The latest upstream changes (presumably #132470) made this pull request unmergeable. Please resolve the merge conflicts.

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 3, 2024
Add bad-reg inline assembly ui test for RISC-V and s390x

rust-lang#131341 (comment)

> Btw, such unsupported registers are present in most architectures, but only aarch64/arm64ec, x86_64, and not yet merged [sparc/sparc64](https://github.com/rust-lang/rust/pull/132472/files#diff-02aebda3376c2b020265137f9ce2c387669ca5cfecd7d60494275c2387db5114) (and powerpc/powerpc64 by this PR) currently have ui tests for them.  I plan to add tests for other arches later.

Starting with RISC-V and s390x, which I'm familiar with and relatively easy to check for correctness.

(Relevant rustc code are supported_types/def_regs/overlapping_regs in [compiler/rustc_target/src/asm/riscv.rs](https://github.com/rust-lang/rust/blob/588a4203508ed7c76750c96b482641261630ed36/compiler/rustc_target/src/asm/riscv.rs) and [compiler/rustc_target/src/asm/s390x.rs](https://github.com/rust-lang/rust/blob/588a4203508ed7c76750c96b482641261630ed36/compiler/rustc_target/src/asm/s390x.rs).)

r? workingjubilee

`@rustbot` label +A-inline-assembly
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 4, 2024
Rollup merge of rust-lang#132516 - taiki-e:asm-ui, r=workingjubilee

Add bad-reg inline assembly ui test for RISC-V and s390x

rust-lang#131341 (comment)

> Btw, such unsupported registers are present in most architectures, but only aarch64/arm64ec, x86_64, and not yet merged [sparc/sparc64](https://github.com/rust-lang/rust/pull/132472/files#diff-02aebda3376c2b020265137f9ce2c387669ca5cfecd7d60494275c2387db5114) (and powerpc/powerpc64 by this PR) currently have ui tests for them.  I plan to add tests for other arches later.

Starting with RISC-V and s390x, which I'm familiar with and relatively easy to check for correctness.

(Relevant rustc code are supported_types/def_regs/overlapping_regs in [compiler/rustc_target/src/asm/riscv.rs](https://github.com/rust-lang/rust/blob/588a4203508ed7c76750c96b482641261630ed36/compiler/rustc_target/src/asm/riscv.rs) and [compiler/rustc_target/src/asm/s390x.rs](https://github.com/rust-lang/rust/blob/588a4203508ed7c76750c96b482641261630ed36/compiler/rustc_target/src/asm/s390x.rs).)

r? workingjubilee

`@rustbot` label +A-inline-assembly
@workingjubilee
Copy link
Member

Thanks!

@bors r=bzEq,workingjubilee

@bors
Copy link
Contributor

bors commented Nov 5, 2024

📌 Commit d19517d has been approved by bzEq,workingjubilee

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2024
tgross35 added a commit to tgross35/rust that referenced this pull request Nov 5, 2024
…rkingjubilee

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

This supports `clobber_abi` which is one of the requirements of stabilization mentioned in rust-lang#93335.

This basically does a similar thing I did in rust-lang#130630 to implement `clobber_abi` for s390x, but for powerpc/powerpc64/powerpc64le.
- This also supports vector registers (as `vreg`) as clobber-only, which need to support clobbering of them to implement `clobber_abi`.
- `vreg` should be able to accept `#[repr(simd)]` types as input/output if the unstable `altivec` target feature is enabled, but `core::arch::{powerpc,powerpc64}` vector types, `#[repr(simd)]`, and `core::simd` are all unstable, so the fact that this is currently a clobber-only should not be considered a blocker of clobber_abi implementation or stabilization. So I have not implemented it in this PR.
  - See rust-lang#131551 (which is based on this PR) for a PR to implement this.
  - (I'm not sticking to whether that PR should be a separate PR or part of this PR, so I can merge that PR into this PR if needed.)

Refs:
- PPC32 SysV: Section "Function Calling Sequence" in [System V Application Binary Interface PowerPC Processor Supplement](https://refspecs.linuxfoundation.org/elf/elfspec_ppc.pdf)
- PPC64 ELFv1: Section 3.2 "Function Calling Sequence" in [64-bit PowerPC ELF Application Binary Interface Supplement](https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-CALL)
- PPC64 ELFv2: Section 2.2 "Function Calling Sequence" in [64-Bit ELF V2 ABI Specification](https://openpowerfoundation.org/specifications/64bitelfabi/)
- AIX: [Register usage and conventions](https://www.ibm.com/docs/en/aix/7.3?topic=overview-register-usage-conventions), [Special registers in the PowerPC®](https://www.ibm.com/docs/en/aix/7.3?topic=overview-special-registers-in-powerpc), [AIX vector programming](https://www.ibm.com/docs/en/aix/7.3?topic=concepts-aix-vector-programming)
- Register definition in LLVM: https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/PowerPC/PPCRegisterInfo.td#L189

If I understand the above four ABI documentations correctly, except for the PPC32 SysV's VR (Vector Registers) and 32-bit AIX (currently not supported by rustc)'s r13, there does not appear to be important differences in terms of implementing `clobber_abi`:
- The above four ABIs are consistent about FPR (0-13: volatile, 14-31: nonvolatile), CR (0-1,5-7: volatile, 2-4: nonvolatile), XER (volatile), and CTR (volatile).
- As for GPR, only the registers we are treating as reserved are slightly different
  - r0, r3-r12 are volatile
  - r1(sp, reserved), r14-31 are nonvolatile
  - r2(reserved) is TOC pointer in PPC64 ELF/AIX, system-reserved register in PPC32 SysV (AFAIK used as thread pointer in Linux/BSDs)
  - r13(reserved for non-32-bit-AIX) is thread pointer in PPC64 ELF, small data area pointer register in PPC32 SysV, "reserved under 64-bit environment; not restored across system calls[^r13]" in AIX)
- As for FPSCR, volatile in PPC64 ELFv1/AIX, some fields are volatile only in certain situations (rest are volatile) in PPC32 SysV/PPC64 ELFv2.
- As for VR (Vector Registers), it is not mentioned in PPC32 SysV, v0-v19 are volatile in both in PPC64 ELF/AIX, v20-v31 are nonvolatile in PPC64 ELF, reserved or nonvolatile depending on the ABI ([vec-extabi vs vec-default in LLVM](https://reviews.llvm.org/D89684), we are [using vec-extabi](rust-lang#131341 (comment))) in AIX:
  > When the default Vector enabled mode is used, these registers are reserved and must not be used.
  > In the extended ABI vector enabled mode, these registers are nonvolatile and their values are preserved across function calls

  I left [FIXME comment about PPC32 SysV](rust-lang#131341 (comment)) and added ABI check for AIX.
- As for VRSAVE, it is not mentioned in PPC32 SysV, nonvolatile in PPC64 ELFv1, reserved in PPC64 ELFv2/AIX
- As for VSCR, it is not mentioned in PPC32 SysV/PPC64 ELFv1, some fields are volatile only in certain situations (rest are volatile) in PPC64 ELFv2, volatile in AIX

We are currently treating r1-r2, r13 (non-32-bit-AIX), r29-r31, LR, CTR, and VRSAVE as reserved.
We are currently not processing anything about FPSCR and VSCR, but I feel those are things that should be processed by `preserves_flags` rather than `clobber_abi` if we need to do something about them. (However, PPCRegisterInfo.td in LLVM does not seem to define anything about them.)

Replaces rust-lang#111335 and rust-lang#124279

cc `@ecnelises` `@bzEq` `@lu-zero`

r? `@Amanieu`

`@rustbot` label +O-PowerPC +A-inline-assembly

[^r13]: callee-saved, according to [LLVM](https://github.com/llvm/llvm-project/blob/6a6af0246bd2d68291582e9aefc0543e5c6102fe/llvm/lib/Target/PowerPC/PPCCallingConv.td#L322) and [GCC](https://github.com/gcc-mirror/gcc/blob/a9173a50e7e346a218323916e4d3add8552529ae/gcc/config/rs6000/rs6000.h#L859).
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2024
Rollup of 9 pull requests

Successful merges:

 - rust-lang#131153 (Improve duplicate derive Copy/Clone diagnostics)
 - rust-lang#131341 (Support clobber_abi and vector registers (clobber-only) in PowerPC inline assembly)
 - rust-lang#132025 (fix suggestion for diagnostic error E0027)
 - rust-lang#132153 (Stabilise `const_char_encode_utf16`.)
 - rust-lang#132303 (More tests for non-exhaustive C-like enums in FFI)
 - rust-lang#132473 ([core/fmt] Replace checked slice indexing by unchecked to support panic-free code)
 - rust-lang#132598 (Clippy: Move some attribute lints to be early pass (post expansion))
 - rust-lang#132606 (Improve example of `impl Pattern for &[char]`)
 - rust-lang#132609 (docs: fix grammar in doc comment at unix/process.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Nov 5, 2024

⌛ Testing commit d19517d with merge 96477c5...

@bors
Copy link
Contributor

bors commented Nov 5, 2024

☀️ Test successful - checks-actions
Approved by: bzEq,workingjubilee
Pushing 96477c5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 5, 2024
@bors bors merged commit 96477c5 into rust-lang:master Nov 5, 2024
7 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 5, 2024
@taiki-e taiki-e deleted the ppc-clobber-abi branch November 5, 2024 06:22
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (96477c5): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 2.7%, secondary -0.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.7% [2.7%, 2.7%] 1
Regressions ❌
(secondary)
2.8% [2.8%, 2.8%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.3% [-4.3%, -4.3%] 1
All ❌✅ (primary) 2.7% [2.7%, 2.7%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 780.214s -> 781.474s (0.16%)
Artifact size: 335.23 MiB -> 335.25 MiB (0.00%)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 7, 2024
Remove fixme comment about clobber_abi on PowerPC

This was considered an unresolved question in rust-lang#131341, but according to the ABI document published in 2011 by Power.org the current implementation is fine as-is.
rust-lang#131341 (comment)

> According to [Power Architecture 32-bit Application Binary Interface Supplement 1.0 - Linux & Embedded](https://web.archive.org/web/20120608163804/https://www.power.org/resources/downloads/Power-Arch-32-bit-ABI-supp-1.0-Unified.pdf) published in 2011, PPC32 has the same convention here as PPC64.
>
> Therefore, we can just remove the FIXME comment here.

r? workingjubilee
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 7, 2024
Rollup merge of rust-lang#132638 - taiki-e:ppc-asm-fixme, r=jieyouxu

Remove fixme comment about clobber_abi on PowerPC

This was considered an unresolved question in rust-lang#131341, but according to the ABI document published in 2011 by Power.org the current implementation is fine as-is.
rust-lang#131341 (comment)

> According to [Power Architecture 32-bit Application Binary Interface Supplement 1.0 - Linux & Embedded](https://web.archive.org/web/20120608163804/https://www.power.org/resources/downloads/Power-Arch-32-bit-ABI-supp-1.0-Unified.pdf) published in 2011, PPC32 has the same convention here as PPC64.
>
> Therefore, we can just remove the FIXME comment here.

r? workingjubilee
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Nov 9, 2024
Support clobber_abi and vector registers (clobber-only) in PowerPC inline assembly

This supports `clobber_abi` which is one of the requirements of stabilization mentioned in #93335.

This basically does a similar thing I did in rust-lang/rust#130630 to implement `clobber_abi` for s390x, but for powerpc/powerpc64/powerpc64le.
- This also supports vector registers (as `vreg`) as clobber-only, which need to support clobbering of them to implement `clobber_abi`.
- `vreg` should be able to accept `#[repr(simd)]` types as input/output if the unstable `altivec` target feature is enabled, but `core::arch::{powerpc,powerpc64}` vector types, `#[repr(simd)]`, and `core::simd` are all unstable, so the fact that this is currently a clobber-only should not be considered a blocker of clobber_abi implementation or stabilization. So I have not implemented it in this PR.
  - See rust-lang/rust#131551 (which is based on this PR) for a PR to implement this.
  - (I'm not sticking to whether that PR should be a separate PR or part of this PR, so I can merge that PR into this PR if needed.)

Refs:
- PPC32 SysV: Section "Function Calling Sequence" in [System V Application Binary Interface PowerPC Processor Supplement](https://refspecs.linuxfoundation.org/elf/elfspec_ppc.pdf)
- PPC64 ELFv1: Section 3.2 "Function Calling Sequence" in [64-bit PowerPC ELF Application Binary Interface Supplement](https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-CALL)
- PPC64 ELFv2: Section 2.2 "Function Calling Sequence" in [64-Bit ELF V2 ABI Specification](https://openpowerfoundation.org/specifications/64bitelfabi/)
- AIX: [Register usage and conventions](https://www.ibm.com/docs/en/aix/7.3?topic=overview-register-usage-conventions), [Special registers in the PowerPC®](https://www.ibm.com/docs/en/aix/7.3?topic=overview-special-registers-in-powerpc), [AIX vector programming](https://www.ibm.com/docs/en/aix/7.3?topic=concepts-aix-vector-programming)
- Register definition in LLVM: https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/PowerPC/PPCRegisterInfo.td#L189

If I understand the above four ABI documentations correctly, except for the PPC32 SysV's VR (Vector Registers) and 32-bit AIX (currently not supported by rustc)'s r13, there does not appear to be important differences in terms of implementing `clobber_abi`:
- The above four ABIs are consistent about FPR (0-13: volatile, 14-31: nonvolatile), CR (0-1,5-7: volatile, 2-4: nonvolatile), XER (volatile), and CTR (volatile).
- As for GPR, only the registers we are treating as reserved are slightly different
  - r0, r3-r12 are volatile
  - r1(sp, reserved), r14-31 are nonvolatile
  - r2(reserved) is TOC pointer in PPC64 ELF/AIX, system-reserved register in PPC32 SysV (AFAIK used as thread pointer in Linux/BSDs)
  - r13(reserved for non-32-bit-AIX) is thread pointer in PPC64 ELF, small data area pointer register in PPC32 SysV, "reserved under 64-bit environment; not restored across system calls[^r13]" in AIX)
- As for FPSCR, volatile in PPC64 ELFv1/AIX, some fields are volatile only in certain situations (rest are volatile) in PPC32 SysV/PPC64 ELFv2.
- As for VR (Vector Registers), it is not mentioned in PPC32 SysV, v0-v19 are volatile in both in PPC64 ELF/AIX, v20-v31 are nonvolatile in PPC64 ELF, reserved or nonvolatile depending on the ABI ([vec-extabi vs vec-default in LLVM](https://reviews.llvm.org/D89684), we are [using vec-extabi](rust-lang/rust#131341 (comment))) in AIX:
  > When the default Vector enabled mode is used, these registers are reserved and must not be used.
  > In the extended ABI vector enabled mode, these registers are nonvolatile and their values are preserved across function calls

  I left [FIXME comment about PPC32 SysV](rust-lang/rust#131341 (comment)) and added ABI check for AIX.
- As for VRSAVE, it is not mentioned in PPC32 SysV, nonvolatile in PPC64 ELFv1, reserved in PPC64 ELFv2/AIX
- As for VSCR, it is not mentioned in PPC32 SysV/PPC64 ELFv1, some fields are volatile only in certain situations (rest are volatile) in PPC64 ELFv2, volatile in AIX

We are currently treating r1-r2, r13 (non-32-bit-AIX), r29-r31, LR, CTR, and VRSAVE as reserved.
We are currently not processing anything about FPSCR and VSCR, but I feel those are things that should be processed by `preserves_flags` rather than `clobber_abi` if we need to do something about them. (However, PPCRegisterInfo.td in LLVM does not seem to define anything about them.)

Replaces #111335 and #124279

cc `@ecnelises` `@bzEq` `@lu-zero`

r? `@Amanieu`

`@rustbot` label +O-PowerPC +A-inline-assembly

[^r13]: callee-saved, according to [LLVM](https://github.com/llvm/llvm-project/blob/6a6af0246bd2d68291582e9aefc0543e5c6102fe/llvm/lib/Target/PowerPC/PPCCallingConv.td#L322) and [GCC](https://github.com/gcc-mirror/gcc/blob/a9173a50e7e346a218323916e4d3add8552529ae/gcc/config/rs6000/rs6000.h#L859).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) merged-by-bors This PR was explicitly merged by bors. O-PowerPC Target: PowerPC processors S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants