Skip to content

powerpc: warn against incorrect values for ABI-relevant target features#157085

Open
RalfJung wants to merge 1 commit into
rust-lang:mainfrom
RalfJung:powerpc-abi-features
Open

powerpc: warn against incorrect values for ABI-relevant target features#157085
RalfJung wants to merge 1 commit into
rust-lang:mainfrom
RalfJung:powerpc-abi-features

Conversation

@RalfJung
Copy link
Copy Markdown
Member

This fills in #131799 for PowerPC. Based on this comment by @beetrees, the relevant target features are "hard-float" and "spe". I confirmed this by looking at the LLVM sources:

  // Set up the register classes.
  addRegisterClass(MVT::i32, &PPC::GPRCRegClass);
  if (!useSoftFloat()) {
    if (hasSPE()) {
      addRegisterClass(MVT::f32, &PPC::GPRCRegClass);
      // EFPU2 APU only supports f32
      if (!Subtarget.hasEFPU2())
        addRegisterClass(MVT::f64, &PPC::SPERCRegClass);
    } else {
      addRegisterClass(MVT::f32, &PPC::F4RCRegClass);
      addRegisterClass(MVT::f64, &PPC::F8RCRegClass);
    }
  }

(this is in llvm/lib/Target/PowerPC/PPCISelLowering.cpp)

So, we make rustc emit a warning indicating the ABI compatibility issues if "spe" or hard-float" gets toggled. The plan is to eventually make this a hard error.

I also found this code there, in the handling for "altivec", that look like they are enabling more registers to be used for the ABI, but maybe I am missing a subtle difference in these addRegisterClass calls:

      if (Subtarget.hasP8Vector())
        addRegisterClass(MVT::f32, &PPC::VSSRCRegClass);

      addRegisterClass(MVT::f64, &PPC::VSFRCRegClass);

      addRegisterClass(MVT::v4i32, &PPC::VSRCRegClass);
      addRegisterClass(MVT::v4f32, &PPC::VSRCRegClass);
      addRegisterClass(MVT::v2f64, &PPC::VSRCRegClass);

Cc @nikic for help with interpreting this LLVM code.

Cc @Gelbpunkt @famfo @neuschaefer as maintainers of affected targets

@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 May 28, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 28, 2026

r? @chenyukang

rustbot has assigned @chenyukang.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 19 candidates

@RalfJung RalfJung changed the title powerpc: warn against correct values for ABI-relevant target features powerpc: warn against incorrect values for ABI-relevant target features May 28, 2026
@RalfJung RalfJung force-pushed the powerpc-abi-features branch from c2a0a73 to 5025c08 Compare May 28, 2026 22:02
@Gelbpunkt
Copy link
Copy Markdown
Contributor

So, we make rustc emit a warning indicating the ABI compatibility issues if "spe" or hard-float" gets toggled. The plan is to eventually make this a hard error.

This is correct to my knowledge, the SPE instructions perform float-point math on the integer registers.

However I'm wondering whether this change will break the SPE targets since to my understanding this enforces hard-float to be enabled and spe to not be enabled for any of the 32-bit PowerPC targets, which includes the existing SPE targets?

@Gelbpunkt
Copy link
Copy Markdown
Contributor

Oh, well, now that I'm looking at the SPE target definitions, they don't even seem to enable SPE in LLVM, so they're probably already broken as-is

@RalfJung
Copy link
Copy Markdown
Member Author

Oh I didn't realize we have SPE targets.

But yeah, looking at those targets... they set -mspe for the linker, but they don't seem to tell LLVM that this should be using SPE? How dos that make any sense? Does LLVM use the target string to figure out the ABI? From the LLVM code, it doesn't look like it.
Cc @BKPepe @biabbas @hax0kartik (you are listed as maintainers for SPE targets)

@RalfJung
Copy link
Copy Markdown
Member Author

This is correct to my knowledge, the SPE instructions perform float-point math on the integer registers.

That wouldn't matter for the ABI. But I guess they will then also use integer registers for passing float arguments / return values, and that makes them ABI-incompatible with non-SPE targets?

@RalfJung RalfJung force-pushed the powerpc-abi-features branch from 5025c08 to a72f0b0 Compare May 29, 2026 07:08
@Gelbpunkt
Copy link
Copy Markdown
Contributor

That wouldn't matter for the ABI. But I guess they will then also use integer registers for passing float arguments / return values, and that makes them ABI-incompatible with non-SPE targets?

Yes, sorry for not being clear on that in my previous reply.

@Gelbpunkt
Copy link
Copy Markdown
Contributor

Gelbpunkt commented May 29, 2026

See also https://www.nxp.com/docs/en/reference-manual/E500ABIUG.pdf

The following algorithm specifies where argument data is passed for the C language [...]

SIMPLE_ARG: A SIMPLE_ARG is one of the following:
— One of the simple integer types no more than 32 bits wide (char, short, int, long, enum)
— A single-precision float
— A pointer to an object of any type
— A struct, union, or long double, any of which shall be treated as a pointer to the
object, or to a copy of the object where necessary to enforce call-by-value
semantics. Only if the caller can ascertain that the object is constant can it pass
a pointer to the object itself.
If gr>10, go to OTHER. Otherwise, load the argument value into general register gr,
set gr to gr+1, and go to SCAN. Values shorter than 32 bits are sign-extended or
zero-extended, depending on whether they are signed or unsigned.

64-bit floats are passed in 2 GPRs.

@rust-log-analyzer
Copy link
Copy Markdown
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
[TIMING:end] tool::ToolBuild { build_compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu, tool: "rustbook", path: "src/tools/rustbook", mode: ToolBootstrap, source_type: Submodule, extra_features: [], allow_features: "", cargo_args: [], artifact_kind: Binary } -- 28.016
[TIMING:end] tool::Rustbook { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu } -- 0.000
    Updating crates.io index
error: failed to get `tower` as a dependency of package `hyper-util v0.1.8`
    ... which satisfies dependency `hyper-util = "^0.1.3"` (locked to 0.1.8) of package `reqwest v0.12.7`
    ... which satisfies dependency `reqwest = "^0.12"` (locked to 0.12.7) of package `trpl v0.3.0 (/checkout/src/doc/book/packages/trpl)`

Caused by:
  failed to load source for dependency `tower`

Caused by:
---
##[endgroup]
[TIMING:start] tool::LibcxxVersionTool { target: x86_64-unknown-linux-gnu }
[TIMING:end] tool::LibcxxVersionTool { target: x86_64-unknown-linux-gnu } -- 0.001
[TIMING:start] toolstate::ToolStateCheck {  }
ERROR: Tool `book` was not recorded in tool state.
ERROR: Tool `nomicon` was not recorded in tool state.
ERROR: Tool `reference` was not recorded in tool state.
ERROR: Tool `rust-by-example` was not recorded in tool state.
ERROR: Tool `edition-guide` was not recorded in tool state.
ERROR: Tool `embedded-book` was not recorded in tool state.
Bootstrap failed while executing `test --stage 2 check-tools`
Build completed unsuccessfully in 0:00:00
  local time: Fri May 29 07:56:59 UTC 2026
  network time: Fri, 29 May 2026 07:56:59 GMT
##[error]Process completed with exit code 1.

@chenyukang
Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot rustbot assigned mu001999 and unassigned chenyukang May 29, 2026
@mu001999
Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot rustbot assigned petrochenkov and unassigned mu001999 May 29, 2026
@petrochenkov
Copy link
Copy Markdown
Contributor

Waiting for the pinged people to respond (for 1-2 weeks max, I guess).
@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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.

7 participants