Skip to content

make pad_i32 of PassMode::cast an integer - #160132

Open
folkertdev wants to merge 1 commit into
rust-lang:mainfrom
folkertdev:pad-i32-count
Open

make pad_i32 of PassMode::cast an integer#160132
folkertdev wants to merge 1 commit into
rust-lang:mainfrom
folkertdev:pad-i32-count

Conversation

@folkertdev

Copy link
Copy Markdown
Contributor

so that we can specify more than one i32 of padding. This PR only adds the functionality but does not yet use it: there should be no functional changes.

This is needed for the ABI of Complex<{ float }> on 32-bit powerpc. Other mechanisms, e.g. using PassMode::prefixed don't appear to work.

More discussion is in #t-compiler/help > power complex abi.

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 29, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@folkertdev
folkertdev force-pushed the pad-i32-count branch 3 times, most recently from 9de525c to 945f5d7 Compare July 29, 2026 10:48
Comment on lines +349 to 351
fn reg_backend_type(&self, ty: &Reg) -> Type<'gcc> {
ty.gcc_type(self)
}

@folkertdev folkertdev Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is extrapolating from the LLVM implementation, it seems to work fine, cc @antoyo if there is something I'm missing.

View changes since the review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be fine.

Comment on lines +58 to +60
/// `pad_i32` indicates how many `Reg::i32()` dummy arguments are emitted before the real
/// argument.
Cast { pad_i32: u8, cast: Box<CastTarget> },

@folkertdev folkertdev Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about the name, but can't really think of anything obviously better but also somewhat short.

View changes since the review

@folkertdev

Copy link
Copy Markdown
Contributor Author

r? @bjorn3
cc @workingjubilee

@folkertdev
folkertdev marked this pull request as ready for review July 29, 2026 12:07
@rustbot

rustbot commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

rustc_codegen_cranelift is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_cranelift instead.

cc @bjorn3

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

rustc_codegen_gcc is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_gcc instead.

cc @antoyo, @GuillaumeGomez

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 29, 2026
@rust-bors

This comment has been minimized.

@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@antoyo

antoyo commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

FYI, there's some issue with cg_gcc and we're about to revert the last sync.

so that we can specify more than one i32 of padding.
@rustbot

rustbot commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

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.

@bjorn3

bjorn3 commented Aug 4, 2026

Copy link
Copy Markdown
Member

This is needed for the ABI of Complex<{ float }> on 32-bit powerpc.

What exactly is the ABI for that type that it needs more than 1 i32 padding?

@folkertdev

Copy link
Copy Markdown
Contributor Author

On powerpc (a 32-bit target), when a Complex<i64> (say) does not fit in the remaining registers, it is moved to the stack (i.e., it is not split between registers and stack) but crucially the remaining registers are consumed. so if there is another i32argument after theComplex` then that must also be passed via the stack. So depending on the number remaining argument registers, you may need to consume up to 3 registers.

@bjorn3

bjorn3 commented Aug 4, 2026

Copy link
Copy Markdown
Member

On powerpc (a 32-bit target), when a Complex (say) does not fit in the remaining registers, it is moved to the stack (i.e., it is not split between registers and stack) but crucially the remaining registers are consumed.

As I understand it, PassMode::Cast should already consume all remaining registers if it is forced to the stack. Is that not the case?

@folkertdev

Copy link
Copy Markdown
Contributor Author

Well so in practice a Complex<i64> or Complex<f64> is actually passed as a [4 x i32] LLVM type, which gives the correct register alignment of 4 (it's weird, but Complex<f32> has a register alignment of 8. this is explicitly documented in GCC) and ensures the value is passed in GPRs, not FPRs.

A [4 x i32] gets split into 4 distinct i32 arguments by LLVM, and so if we don't explicitly occupy the remaining GPR registers then the value is in fact split between registers and stack.

@folkertdev

Copy link
Copy Markdown
Contributor Author

Something that sort of works is using a simd type, because it does not get split. But, it (correctly) warns about missing target features, and scheme probably fails when there are actually vector registers available.

So, yeah, I think this change is needed to support that ABI. it's still pretty limited, Clang allows you to specify a whole custom type for padding.

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

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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.

5 participants