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

Rollup of 8 pull requests #106318

Closed
wants to merge 22 commits into from
Closed

Rollup of 8 pull requests #106318

wants to merge 22 commits into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 31, 2022

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

pcwalton and others added 22 commits November 17, 2022 09:53
…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)
Signed-off-by: Yuki Okushi <[email protected]>
Previously, clean only supported `--stage 0` for specific crates.

The new `crate_description` function generates a string that looks
like
```
: {rustc_query_impl}
```
This will prevent users with the pre-push hook from pushing a merge
commit.

Exceptions are added for subtree updates. These exceptions are a little
hacky and may be non-exhaustive but can be extended in the future.
Before:
```
Testing ["rustc_interface"] stage0 (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu)
```

After:
```
Testing {rustc_interface} stage0 (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu)
```

Note there is a slight consistency between `build` and `test`: The
former doesn't print "compiler artifacts". It would be annoying to fix
and doesn't hurt anything, so I left it be.

```
; x t rustc_interface --stage 0 --dry-run
Testing {rustc_interface} stage0 (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu)
; x b rustc_interface --stage 0 --dry-run
Building {rustc_interface} stage0 compiler artifacts (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu)
```
This makes it easier to see them (and makes people go owo).
rustc_target: Add alignment to indirectly-passed by-value types, correcting 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)
…-only-bors-is-allowed-to-do-that, r=jyn514

Add tidy check to deny merge commits

This will prevent users with the pre-push hook from pushing a merge commit.

Exceptions are added for subtree updates. These exceptions are a little hacky and may be non-exhaustive but can be extended in the future.

I added a link to `@jyn514's` blog post for the error case because that's the best resource to solve merge commits. But it would probably be better if it was integrated into https://rustc-dev-guide.rust-lang.org/git.html#no-merge-policy, then we could link that instead.

r? `@jyn514`
Add regression test for rust-lang#86106

Closes rust-lang#86106
r? `@nikic`

Signed-off-by: Yuki Okushi <[email protected]>
Make tidy errors red

This makes it easier to see them (and makes people go owo).

I also changes the error codes check to not print too many things and use `tidy_error`.

r? `@jyn514`
…ples-layout-test, r=notriddle

Extend scraped examples layout GUI test for position of buttons

This is a regression test for rust-lang#106279.

r? ``@notriddle``
 bootstrap: Get rid of tail_args in stream_cargo

Based on rust-lang#106303 for convenience.

r? ``@Mark-Simulacrum``
Dont use `--merge-base` during bootstrap formatting subcommand

I use a development image with Ubuntu 20.04 LTS, which has git 2.25.

Recently, `./x.py test tidy --bless` regressed in rust-lang#105702 because it uses the `--merge-base` option on `diff-index`, which was only introduced in git 2.30 (git/git@0f5a1d4). Luckily, it can be replicated via two calls to `git merge-base` + `git diff-index`, so let's just use that.
Fix panic on `x build --help`

Fixes rust-lang#106313. This avoids trying to run `get_help` unless we actually need to see the paths that are available for the subcommand.

This originally regressed in rust-lang#106166.
@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc 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. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Dec 31, 2022
@jyn514
Copy link
Member Author

jyn514 commented Dec 31, 2022

bors looks very confused. not all of these have been approved.

@jyn514 jyn514 closed this Dec 31, 2022
@jyn514 jyn514 deleted the rollup-8aavrrl branch December 31, 2022 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants