AIX: use align 8 for byval parameter#134396
Conversation
|
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
rustbot
added
S-waiting-on-review
mustartt
changed the title
|
...wait, so on AIX, loads from the stack aren't aligned to the size of the type? whee... |
Not exactly the case. That's just how we are representing it in the IR. In reality, it almost always 16 byte aligned. |
|
??? what is the actuality then? |
This sounds like a statement for 32-bit mode. TMK, in 64-bit mode, Clang produces 8-byte alignment. |
mustartt
force-pushed
the
byval-pointer-natural-alignment
branch
from
51dcfae to
7bfcddf
Compare
mustartt
changed the title
mustartt
changed the title
For input values of the parameter, yes, the loads from the stack can be not aligned to the type. But the function local have to have proper alignment.
We do not (at the assembly level) pass pointers to the callee. The incoming parameter is represented as a pointer in the IR (associated to register-sized/aligned memory when not stored in the registers themselves). That is: The alignment is associated with the size of registers (except where a parameter type has an non-packed vector in it). |
@mustartt, Clang creates the function local separately, does the Rust front-end do so as well? |
Yes, the function locals will have the type's alignment. For example: define i64 @rust_func(ptr byval([16 x i8]) align 8 %0) unnamed_addr #0 personality ptr @rust_eh_personality {
start:
%val = alloca [16 x i8], align 16
call void @llvm.memcpy.p0.p0.i64(ptr align 16 %val, ptr align 8 %0, i64 16, i1 false)
...
} |
| // The AIX ABI expect byval for aggregates | ||
| // See https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/Targets/PPC.cpp. | ||
| // The incoming parameter is represented as a pointer in the IR, | ||
| // the alignment is associated with the size of the register. (align 8 for 64bit) |
There was a problem hiding this comment.
Should there be a TODO regarding PowerPC SIMD vector types (https://doc.rust-lang.org/core/arch/powerpc64/index.html)?
There was a problem hiding this comment.
Probably. Its 16 for vector types?
It's 16 for vector types and aggregates with (non-packed) vector subobjects. There seems to be a bug in Clang for the packed cases.
|
I know nothing about this :) |
wesleywiser
left a comment
wesleywiser
left a comment
There was a problem hiding this comment.
This looks correct to me. Let's cc the AIX target maintainers though: @daltenty @gilamn5tr
|
One more friendly reminder to the target maintainers to see if they spot an issue with this before merging: @daltenty @gilamn5tr |
|
@wesleywiser We're happy with it for now. LGTM! |
|
Thanks for taking a look @gilamn5tr! @bors r+ |
bors
added
S-waiting-on-bors
daltenty
left a comment
daltenty
left a comment
There was a problem hiding this comment.
Confirming this looks good from the AIX target perspective
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#133372 (Refactor dyn-compatibility error and suggestions) - rust-lang#134396 (AIX: use align 8 for byval parameter) - rust-lang#135156 (Make our `DIFlags` match `LLVMDIFlags` in the LLVM-C API) - rust-lang#135816 (Use `structurally_normalize` instead of manual `normalizes-to` goals in alias relate errors) - rust-lang#135823 (make UI tests that use `--test` work on panic=abort targets) - rust-lang#135850 (Update the `wasm-component-ld` tool) - rust-lang#135858 (rustdoc: Finalize dyn compatibility renaming) - rust-lang#135866 (Don't pick `T: FnPtr` nested goals as the leaf goal in diagnostics for new solver) - rust-lang#135874 (Enforce that all spans are lowered in ast lowering) - rust-lang#135875 (Remove `Copy` bound from `enter_forall`) r? `@ghost` `@rustbot` modify labels: rollup
10 participants
On AIX, byval pointer arguments are aligned to 8 bytes based on the 64bit register size. For example, the C callee https://godbolt.org/z/5f4vnG6bh will expect the following argument.
This case is captured by
run-make/extern-fn-explicit-align