Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resurrect: rustc_target: Add alignment to indirectly-passed by-value types, correcting the alignment of byval on x86 in the process. #112157

Merged
merged 26 commits into from
Jul 15, 2023

Commits on Jul 10, 2023

  1. rustc_target: Add alignment to indirectly-passed by-value types, corr…

    …ecting the
    
    alignment of `byval` on x86 in the process.
    
    Commit 88e4d2c 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 rust-lang#80127, because it will make the `align`
    parameter attribute for `byval` parameters match the platform ABI on LLVM
    x86-64.
    
    [this comment]: rust-lang#80822 (comment)
    pcwalton authored and erikdesjardins committed Jul 10, 2023
    Configuration menu
    Copy the full SHA
    0becc89 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    1022926 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    be1d4e3 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    a07eb0a View commit details
    Browse the repository at this point in the history
  5. add ignore-cross-compile to run-make/extern-fn-explicit-align

    From the test logs, other extern-fn-* tests have this:
    
    [run-make] tests/run-make/extern-fn-with-packed-struct ... ignored, ignored when cross-compiling
    [run-make] tests/run-make/extern-fn-with-union ... ignored, ignored when cross-compiling
    [run-make] tests/run-make/extern-multiple-copies ... ignored, ignored when cross-compiling
    [run-make] tests/run-make/extern-multiple-copies2 ... ignored, ignored when cross-compiling
    [run-make] tests/run-make/extern-overrides-distribution ... ignored, ignored when cross-compiling
    [run-make] tests/run-make/extra-filename-with-temp-outputs ... ignored, ignored when cross-compiling
    [run-make] tests/run-make/extern-fn-explicit-align ... FAILED
    erikdesjardins committed Jul 10, 2023
    Configuration menu
    Copy the full SHA
    0f7d333 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    fdaaf86 View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    84ff2e3 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    5f4472e View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    bc9d26a View commit details
    Browse the repository at this point in the history
  10. align-byval test: add x86

    x86 Windows also should not use byval since the struct is
    overaligned, see https://reviews.llvm.org/D72114
    erikdesjardins committed Jul 10, 2023
    Configuration menu
    Copy the full SHA
    08d1892 View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    8ec90f6 View commit details
    Browse the repository at this point in the history
  12. extern-fn-struct-passing-abi test: ensure we don't start passing stru…

    …ct with natural alignment > 8 by reference
    erikdesjardins committed Jul 10, 2023
    Configuration menu
    Copy the full SHA
    7089321 View commit details
    Browse the repository at this point in the history
  13. Configuration menu
    Copy the full SHA
    ed317e4 View commit details
    Browse the repository at this point in the history
  14. Configuration menu
    Copy the full SHA
    209ed07 View commit details
    Browse the repository at this point in the history
  15. Configuration menu
    Copy the full SHA
    0e76446 View commit details
    Browse the repository at this point in the history
  16. Configuration menu
    Copy the full SHA
    f704396 View commit details
    Browse the repository at this point in the history
  17. Configuration menu
    Copy the full SHA
    65d11b5 View commit details
    Browse the repository at this point in the history
  18. Configuration menu
    Copy the full SHA
    00b3eca View commit details
    Browse the repository at this point in the history
  19. Configuration menu
    Copy the full SHA
    4c1dbc3 View commit details
    Browse the repository at this point in the history
  20. Configuration menu
    Copy the full SHA
    2591c30 View commit details
    Browse the repository at this point in the history
  21. Configuration menu
    Copy the full SHA
    7e933b4 View commit details
    Browse the repository at this point in the history
  22. Configuration menu
    Copy the full SHA
    d1e764c View commit details
    Browse the repository at this point in the history
  23. Configuration menu
    Copy the full SHA
    c858d34 View commit details
    Browse the repository at this point in the history

Commits on Jul 13, 2023

  1. extern fn-explicit-align test: don't use uint128_t

    ...which seems not to be available on some platforms.
    Or maybe it is under a different name but I don't want to deal with that
    
    Instead, use two u64s. This isn't exactly the same, but we already have
    some coverage of the packed u128 case in another test, so it's not
    essential to have it here.
    erikdesjardins committed Jul 13, 2023
    Configuration menu
    Copy the full SHA
    ecf2390 View commit details
    Browse the repository at this point in the history

Commits on Jul 14, 2023

  1. Configuration menu
    Copy the full SHA
    f297f32 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    2daacf5 View commit details
    Browse the repository at this point in the history