Rollup of 9 pull requests #123645
Merged
Rollup of 9 pull requests #123645
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I happened to notice today that there's actually two such calls emitted in the assembly: <https://rust.godbolt.org/z/1Wbbd3Ts6> Since they're impossible, hopefully telling LLVM that will also help optimizations elsewhere.
This reverts commit 399a258.
This particular cast appears to have been copied over from clang, but there are plenty of other call sites in clang that don't bother with a cast here, and it works fine without one. For context, `llvm::Intrinsic::ID` is a typedef for `unsigned`, and `llvm::Intrinsic::instrprof_increment` is a member of `enum IndependentIntrinsics : unsigned`.
When passing a 16 (or higher) aligned struct by value on ppc64le, it needs to be passed as an array of `i128` rather than an array of `i64`. This will force the use of an even starting register. For the case of a 16 byte struct with alignment 16 it is important that `[1 x i128]` is used instead of `i128` -- apparently, the latter will get treated similarly to `[2 x i64]`, not exhibiting the correct ABI. Add a `force_array` flag to `Uniform` to support this. The relevant clang code can be found here: https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L878-L884 https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L780-L784 I think the corresponding psABI wording is this: > Fixed size aggregates and unions passed by value are mapped to as > many doublewords of the parameter save area as the value uses in > memory. Aggregrates and unions are aligned according to their > alignment requirements. This may result in doublewords being > skipped for alignment. In particular the last sentence. Fixes rust-lang#122767.
The actual ABI implication here is that in some cases the values are required to be "consecutive", i.e. must either all be passed in registers or all on stack (without padding). Adjust the code to either use Uniform::new() or Uniform::consecutive() depending on which behavior is needed. Then, when lowering this in LLVM, skip the [1 x i128] to i128 simplification if is_consecutive is set. i128 is the only case I'm aware of where this is problematic right now. If we find other cases, we can extend this (either based on target information or possibly just by not simplifying for is_consecutive entirely).
`lexer::UnmatchedDelim` struct in `rustc_parse` is unnecessary public outside of the crate. This commit reduces the visibility to `pub(crate)`. Beside, this removes unnecessary field `expected_delim` that causes warnings after changing the visibility.
In its first step of computing transmutability, `rustc_transmutability` constructs a byte-level representation of type layout (`Tree`). Previously, this representation was computed for ADTs by inspecting the ADT definition and performing our own layout computations. This process was error-prone, verbose, and limited our ability to analyze many types (particularly default-repr types). In this PR, we instead construct `Tree`s from `rustc_target::abi::Layout`s. This helps ensure that layout optimizations are reflected our analyses, and increases the kinds of types we can now analyze, including: - default repr ADTs - transparent unions - `UnsafeCell`-containing types Overall, this PR expands the expressvity of `rustc_transmutability` to be much closer to the transmutability analysis performed by miri. Future PRs will work to close the remaining gaps (e.g., support for `Box`, raw pointers, `NonZero*`, coroutines, etc.).
We oddly weren't testing the more usual case of casting non-methods to function pointers. The KCFI shim insertion logic would ICE on these due to asking for an irrefutable associated item if we cast a function to a function pointer without needing a traditional shim.
Fix argument ABI for overaligned structs on ppc64le When passing a 16 (or higher) aligned struct by value on ppc64le, it needs to be passed as an array of `i128` rather than an array of `i64`. This will force the use of an even starting doubleword. For the case of a 16 byte struct with alignment 16 it is important that `[1 x i128]` is used instead of `i128` -- apparently, the latter will get treated similarly to `[2 x i64]`, not exhibiting the correct ABI. Add a `force_array` flag to `Uniform` to support this. The relevant clang code can be found here: https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L878-L884 https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L780-L784 I think the corresponding psABI wording is this: > Fixed size aggregates and unions passed by value are mapped to as > many doublewords of the parameter save area as the value uses in > memory. Aggregrates and unions are aligned according to their > alignment requirements. This may result in doublewords being > skipped for alignment. In particular the last sentence. Though I didn't find any wording for Clang's behavior of clamping the alignment to 16. Fixes rust-lang#122767. r? `@cuviper`
Safe Transmute: Compute transmutability from `rustc_target::abi::Layout` In its first step of computing transmutability, `rustc_transmutability` constructs a byte-level representation of type layout (`Tree`). Previously, this representation was computed for ADTs by inspecting the ADT definition and performing our own layout computations. This process was error-prone, verbose, and limited our ability to analyze many types (particularly default-repr types). In this PR, we instead construct `Tree`s from `rustc_target::abi::Layout`s. This helps ensure that layout optimizations are reflected our analyses, and increases the kinds of types we can now analyze, including: - default repr ADTs - transparent unions - `UnsafeCell`-containing types Overall, this PR expands the expressvity of `rustc_transmutability` to be much closer to the transmutability analysis performed by miri. Future PRs will work to close the remaining gaps (e.g., support for `Box`, raw pointers, `NonZero*`, coroutines, etc.). r? `@compiler-errors`
…li-obk Fix `ByMove` coroutine-closure shim (for 2021 precise closure capturing behavior) This PR reworks the way that we perform the `ByMove` coroutine-closure shim to account for the fact that the upvars of the outer coroutine-closure and the inner coroutine might not line up due to edition-2021 closure capture rules changes. Specifically, the number of upvars may differ *and/or* the inner coroutine may have additional projections applied to an upvar. This PR reworks the information we pass into the `ByMoveBody` MIR visitor to account for both of these facts. I tried to leave comments explaining exactly what everything is doing, but let me know if you have questions. r? oli-obk
bootstrap: remove unused pub fns Looks dead, remove.
Don't emit divide-by-zero panic paths in `StepBy::len` I happened to notice today that there's actually two such calls emitted in the assembly: <https://rust.godbolt.org/z/1Wbbd3Ts6> Since they're impossible, hopefully telling LLVM that will also help optimizations elsewhere.
…errors Restore `pred_known_to_hold_modulo_regions` As requested by `@lcnr` in rust-lang#123275 (comment) this PR restores `pred_known_to_hold_modulo_regions` to fix that "unexpected unsized tail" beta regression. This also adds the reduced repro from rust-lang#123275 (comment) as a sub-optimal test is better than no test at all, and it'll also cover rust-lang#108721. It still ICEs on master, even though https://github.com/phlip9/rustc-warp-ice doesn't on nightly anymore, since rust-lang#122493. Fixes rust-lang#123275. r? `@compiler-errors` but feel free to close if you'd rather have a better test instead cc `@wesleywiser` who had signed up to do the revert Will need a backport if we go with this PR: `@rustbot` label +beta-nominated
Remove unnecessary cast from `LLVMRustGetInstrProfIncrementIntrinsic` (Noticed while reviewing rust-lang#123409.) This particular cast appears to have been copied over from clang, but there are plenty of other call sites in clang that don't bother with a cast here, and it works fine without one. For context, `llvm::Intrinsic::ID` is a typedef for `unsigned`, and `llvm::Intrinsic::instrprof_increment` is a member of `enum IndependentIntrinsics : unsigned`. --- The formatting change in `unwrap(M)` is the result of manually running `clang-format` on this file, and then reverting all changes other than the ones affecting these lines.
…ity, r=compiler-errors parser: reduce visibility of unnecessary public `UnmatchedDelim` `lexer::UnmatchedDelim` struct in `rustc_parse` is unnecessary public outside of the crate. This commit reduces the visibility to `pub(crate)`. Beside, this removes unnecessary field `expected_delim` that causes warnings after changing the visibility.
…rrors CFI: Fix ICE in KCFI non-associated function pointers We oddly weren't testing the more usual case of casting non-methods to function pointers. The KCFI shim insertion logic would ICE on these due to asking for an irrefutable associated item if we cast a function to a function pointer without needing a traditional shim. r? `@compiler-errors`
rustbot
added
PG-exploit-mitigations
|
@bors r+ rollup=never p=9 |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR:
previous master: 211518e5fb In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
|
Finished benchmarking commit (ab5bda1): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 668.886s -> 669.403s (0.08%) |
|
Looks like a temporary spike. Marking as triaged. @rustbot label: +perf-regression-triaged |
rustbot
added
the
perf-regression-triaged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
rustc_target::abi::Layout#123367 (Safe Transmute: Compute transmutability fromrustc_target::abi::Layout)ByMovecoroutine-closure shim (for 2021 precise closure capturing behavior) #123518 (FixByMovecoroutine-closure shim (for 2021 precise closure capturing behavior))StepBy::len#123564 (Don't emit divide-by-zero panic paths inStepBy::len)pred_known_to_hold_modulo_regions#123578 (Restorepred_known_to_hold_modulo_regions)LLVMRustGetInstrProfIncrementIntrinsic#123591 (Remove unnecessary cast fromLLVMRustGetInstrProfIncrementIntrinsic)UnmatchedDelim#123632 (parser: reduce visibility of unnecessary publicUnmatchedDelim)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup