Skip to content

powerpc64-ibm-aix: fix cfg(target_abi) value - #162325

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

powerpc64-ibm-aix: fix cfg(target_abi) value#162325
RalfJung wants to merge 1 commit into
rust-lang:mainfrom
RalfJung:aix-abi

Conversation

@RalfJung

@RalfJung RalfJung commented Sep 5, 2026

Copy link
Copy Markdown
Member

Our powerpc64-ibm-aix target currently sets cfg_abi to "vec-extabi", which means that programs compiled for this target think they use that ABI. Even our inline asm logic trusts this field. But that's a lie, we're actually using the default ABI since we are never setting EnableAIXExtendedAltivecABI on the LLVM side. We should fix that discrepancy.

Between changing how we generate the code, and changing the label we put into cfg_abi, the latter is the less risky change. So let's do that.

This has been tried before in #153830. We then decided to wait a bit while the target maintainers investigate whether they want to change the ABI or not. I think we have waited long enough. The last comment from them that I found is this one which says "I'd conclude the extend vector ABI should be disabled for now"; there have been further questions but no further communication. We can always still change the ABI in the future, but for now let's fix the obvious bug where the ABI we report in cfg_abi does not match the actual ABI we are compiling for.

Cc @Gelbpunkt @daltenty @gilamn5tr @amy-kwan @taiki-e

@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 Sep 5, 2026
@rustbot

rustbot commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
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 75 candidates
  • Random selection from 20 candidates

// FIXME: using `cfg_abi` here is wrong -- that's not a source of truth, it's just
// what we display to the user. Instead we need to check the knob that controls
// whether the ABI is actually used. Currently there is no such knob, a Rust target
// can therefore never actually use `vec-extabi`.

@RalfJung RalfJung Sep 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Given this, IMO we should just always error here, and never allow these registers to be used on AIX. This would remove the last cfg_abi check in the asm logic, which is good -- hopefully we'll never accidentally add any new such checks in the future. :)
@taiki-e @Amanieu would that be okay?

View changes since the review

@rust-log-analyzer

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants