Rollup of 4 pull requests#155224
Closed
JonathanBrouwer wants to merge 13 commits intorust-lang:mainfrom
Closed
Conversation
…pable Some optimization passes _improve_ compile times [1]. So we want to run some passes even with `-Copt-level=0`. That means that some of the lines in the test can be optimized away. To make regression testing more robust, we also want to run the test with such passes disabled. The solution is to use two revisions. One with default `-Copt-level=0` passes, and one "even less optimized", with enough optimization passes disabled to keep the maximum number of lines steppable. [1]: rust-lang/compiler-team#319
…tput Add a flag to control verbose subprocess output for run-make tests. When using --no-capture on panic=abort test suites like cg_clif, passing test output can flood the terminal. This flag (default true) lets users opt out via --verbose-run-make-subprocess-output=false. Extract a private print_command_output helper in run-make-support to avoid inlining the output logic in handle_failed_output, ensuring failures always print regardless of the flag.
Change codegen of LLVM intrinsics to be name-based, and add llvm linkage support for `bf16(xN)` and `i1xN` *[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust/pull/140763)* This PR changes how LLVM intrinsics are codegen # Explanation of the changes ## Current procedure This is the same for all functions, LLVM intrinsics are _not_ treated specially - We get the LLVM Type of a function simply using the argument types. For example, the following function ```rust #[link_name = "llvm.sqrt.f32"] fn sqrtf32(a: f32) -> f32; ``` will have LLVM type simply `f32 (f32)` due to the Rust signature ### Pros - Simpler to implement, no extra complexity involved due to LLVM intrinsics ### Cons - LLVM intrinsics have a well-defined signature, completely defined by their name (and if it is overloaded, the type parameters). So, this process of converting Rust signatures to LLVM signatures may not work, for example the following code generates LLVM IR without any problem ```rust #[link_name = "llvm.sqrt.f32"] fn sqrtf32(a: i32) -> f32; ``` but the generated LLVM IR is invalid, because it has wrong signature for the intrinsic ([Godbolt](https://godbolt.org/z/6ff9hrcd5), adding `-Zverify-llvm-ir` to it will fail compilation). I would expect this code to not compile at all instead of generating invalid IR. - LLVM intrinsics that have types in their signature that can't be accessed from Rust (notable examples are the AMX intrinsics that have the `x86amx` type, and (almost) all intrinsics that have vectors of `i1` types) can't be linked to at all. This is a (major?) roadblock in the AMX and AVX512 support in stdarch. - If code uses an non-existing LLVM intrinsic, even `-Zverify-llvm-ir` won't complain. Eventually it will error out due to the non-existing function (courtesy of the linker). I don't think this is a behavior we want. ## What this PR does - When linking to **non-overloaded** intrinsics, we use the function `LLVMIntrinsicGetType` to directly get the function type of the intrinsic from LLVM. - We then use this LLVM definition to _verify_ the Rust signature, and emit a proper error if it doesn't match, instead of silently emitting invalid IR. - Lint if linking to deprecated or invalid LLVM intrinsics > [!NOTE] > This PR only focuses on non-overloaded intrinsics, overloaded can be done in a future PR Regardless, the undermentioned functionalities work for **all** intrinsics - If we can't find the intrinsic, we check if it has been `AutoUpgrade`d by LLVM. If not, that means it is an invalid intrinsic, and we error out. - Don't allow intrinsics from other archs to be declared, e.g. error out if an AArch64 intrinsic is declared when we are compiling for x86 ### Pros - It is now not possible (or at least, it would require _significantly_ more leaps and bounds) to introduce invalid IR using **non-overloaded** LLVM intrinsics. - As we are now doing the matching of Rust signatures to LLVM intrinsics ourselves, we can now add bypasses to enable linking to such non-Rust types (e.g. matching 8192-bit vectors to `x86amx` and injecting `llvm.x86.cast.vector.to.tile` and `llvm.x86.cast.tile.to.vector`s in callsite) > [!NOTE] > I don't intend for these bypasses to be permanent. A better approach will be introducing a `bf16` type in Rust, and allowing `repr(simd)` with `bool`s to get Rust-native `i1xN`s. These are meant to be short-time, as I mentioned, "bypass"es. They shouldn't cause any major breakage even if removed, as `link_llvm_intrinsics` is perma-unstable. This PR adds bypasses for `bf16` (via `i16`), `bf16xN` (via `i16xN`) and `i1xN` (via `iM`, where `M` is the smallest power of 2 s.t. `M >= N`, unless `N <= 4`, where we use `M = 8`). This will unblock AVX512-VP2INTERSECT and a lot of bf16 intrinsics in stdarch. This PR also automatically destructures structs if the types don't exactly match (this is required for us to start emitting hard errors on mismmatches). ### Cons - This only works for non-overloaded intrinsics (at least for now). Improving this to work with overloaded intrinsics too will involve significantly more work. # Possible ways to extend this to overloaded intrinsics (future) ## Parse the mangled intrinsic name to get the type parameters LLVM has a stable mangling of intrinsic names with type parameters (in `LLVMIntrinsicCopyOverloadedName2`), so we can parse the name to get the type parameters, and then just do the same thing. ### Pros - For _most_ intrinsics, this will work perfectly, and is a easy way to do this. ### Cons - The LLVM mangling is not perfectly reversible. When we have `TargetExt` types or identified structs, their name is a part of the mangling, making it impossible to reverse. Even more complexities arise when there are unnamed identified structs, as LLVM adds more mangling to the names. - @nikic's work on LLVM intrinsics will remove the name mangling, making this approach impossible ## Use the `IITDescriptor` table and the Rust function signature We can use the base name to get the `IITDescriptor`s of the corresponding intrinsic, and then manually implement the _matching_ logic based on the Rust signature. ### Pros - Doesn't have the above mentioned limitation of the parsing approach, has correct behavior even when there are identified structs and `TargetExt` types. Also, fun fact, Rust exports all struct types as literal structs (unless it is emitting LLVM IR, then it always uses named identified structs, with mangled names) ### Cons - **Doesn't** actually use the type parameters in the name, only uses the base name and the Rust signature to get the llvm signature (although we _can_ check that it is the correct name). It means there would be no way to (for example) link against `llvm.sqrt.bf16` until we have `bf16` types in Rust. Because if we are using `u16`s (or any other type) as `bf16`s, then the matcher will deduce that the signature is `u16 (u16)` not `bf16 (bf16)` (which would lead to an error because `u16` is not a valid type parameter for `llvm.sqrt`), even though the intended type parameter is specified in the name. - Much more complex, and hard to maintain as LLVM gets new `IITDescriptorKind`s These 2 approaches might give different results for same function. Let's take ```rust #[link_name = "llvm.is.constant.bf16"] fn foo(a: u16) -> bool ``` The name-based approach will decide that the type parameter is `bf16`, and the LLVM signature is `i1 (bf16)` and will inject some bitcasts at callsite. The `IITDescriptor`-based approach will decide that the LLVM signature is `i1 (u16)`, and will see that the name given doesn't match the expected name (`llvm.is.constant.u16`), and will error out. Reviews are welcome, as this is my first time _actually_ contributing to `rustc` @rustbot label T-compiler A-codegen A-LLVM r? codegen
tests/debuginfo/basic-stepping.rs: Explain why all lines are not steppable Some optimization passes [_improve_](rust-lang/compiler-team#319) compile times. So we want to run some passes even with `-Copt-level=0`. That means that some debuggable lines can be optimized away. Document that as expected behavior. Closes rust-lang#33013. Replaces rust-lang#151426. See that PR for some discussion.
…r=jieyouxu Add --verbose-run-make-subprocess-output flag to suppress verbose run-make output for passing tests - Adds `--verbose-run-make-subprocess-output` flag to `./x test` and compiletest - `./x test --no-capture --verbose-run-make-subprocess-output=false` suppresses verbose subprocess output for passing run-make tests - Failed tests always print their output regardless of `--verbose-run-make-subprocess-output` - Default behavior (verbose) is unchanged This addresses the request from @bjorn3 which needs `--no-capture` (due to `panic=abort`) but doesn't want output dumped for every passing test. Helps with rust-lang#154069 ## Test plan - [x] `./x test tests/run-make/bare-outfile --no-capture --force-rerun` — verbose output for passing test - [x] `./x test tests/run-make/bare-outfile --no-capture --verbose-run-make-subprocess-output=false --force-rerun` — no verbose output for passing test - [x] Failing test still dumps output with `--verbose-run-make-subprocess-output=false`
…ueries, r=jackh726 Small refactor of `arena_cache` query values Query modifier `arena_cache` automatically allocates only `Some` variant of query's value of type `Option<&'tcx T>`, so `mir_callgraph_cyclic` should've been doing it from the beginning. The same could be said for any `Result<&'tcx T, ErrorGuaranteed>` as `ErrorGuaranteed` is a zero sized type, but that requires adding a new `ArenaCached` implementation.
rustbot
added
A-compiletest
Contributor
Author
|
@bors r+ rollup=never p=5 |
Contributor
rust-bors
bot
added
S-waiting-on-bors
rust-bors
bot
added
S-waiting-on-author
Contributor
|
This pull request was unapproved due to being closed. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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:
bf16(xN)andi1xN#140763 (Change codegen of LLVM intrinsics to be name-based, and add llvm linkage support forbf16(xN)andi1xN)arena_cachequery values #154944 (Small refactor ofarena_cachequery values)r? @ghost
Create a similar rollup