Commit Graph

9 Commits

Author SHA1 Message Date
Erik Desjardins
2daacf5af9 i686-windows: make requested alignment > 4 special case apply transitively 2023-07-14 17:48:13 -04:00
Erik Desjardins
7e933b4e26 repr(align) <= 4 should still be byval 2023-07-10 19:19:40 -04:00
Erik Desjardins
f704396c0e align-byval test: add cases for lower requested alignment, wrapped, and repr(transparent) 2023-07-10 19:19:39 -04:00
Erik Desjardins
0e76446a9f ensure byval allocas are sufficiently aligned 2023-07-10 19:19:38 -04:00
Erik Desjardins
209ed071ba align-byval test: add cases for <= align 4 2023-07-10 19:19:38 -04:00
Erik Desjardins
8ec90f6f14 align-byval test: add cases distinguishing natural vs forced/requested alignment 2023-07-10 19:19:37 -04:00
Erik Desjardins
08d18929fb align-byval test: add x86
x86 Windows also should not use byval since the struct is
overaligned, see https://reviews.llvm.org/D72114
2023-07-10 19:19:37 -04:00
Erik Desjardins
102292655b align-byval test: use revisions to test different targets 2023-07-10 19:19:35 -04:00
Patrick Walton
0becc89d4a rustc_target: Add alignment to indirectly-passed by-value types, correcting the
alignment of `byval` on x86 in the process.

Commit 88e4d2c291 from five years ago removed
support for alignment on indirectly-passed arguments because of problems with
the `i686-pc-windows-msvc` target. Unfortunately, the `memcpy` optimizations I
recently added to LLVM 16 depend on this to forward `memcpy`s. This commit
attempts to fix the problems with `byval` parameters on that target and now
correctly adds the `align` attribute.

The problem is summarized in [this comment] by @eddyb. Briefly, 32-bit x86 has
special alignment rules for `byval` parameters: for the most part, their
alignment is forced to 4. This is not well-documented anywhere but in the Clang
source. I looked at the logic in Clang `TargetInfo.cpp` and tried to replicate
it here. The relevant methods in that file are
`X86_32ABIInfo::getIndirectResult()` and
`X86_32ABIInfo::getTypeStackAlignInBytes()`. The `align` parameter attribute
for `byval` parameters in LLVM must match the platform ABI, or miscompilations
will occur. Note that this doesn't use the approach suggested by eddyb, because
I felt it was overkill to store the alignment in `on_stack` when special
handling is really only needed for 32-bit x86.

As a side effect, this should fix #80127, because it will make the `align`
parameter attribute for `byval` parameters match the platform ABI on LLVM
x86-64.

[this comment]: https://github.com/rust-lang/rust/pull/80822#issuecomment-829985417
2023-07-10 19:19:30 -04:00