Rollup of 6 pull requests #125463
Rollup of 6 pull requests #125463
Conversation
Typical uses of ThinLTO don't have any use for this as a standalone file, but distributed ThinLTO uses this to make the linker phase more efficient. With clang you'd do something like `clang -flto=thin -fthin-link-bitcode=foo.indexing.o -c foo.c` and then get both foo.o (full of bitcode) and foo.indexing.o (just the summary or index part of the bitcode). That's then usable by a two-stage linking process that's more friendly to distributed build systems like bazel, which is why I'm working on this area. I talked some to @teresajohnson about naming in this area, as things seem to be a little confused between various blog posts and build systems. "bitcode index" and "bitcode summary" tend to be a little too ambiguous, and she tends to use "thin link bitcode" and "minimized bitcode" (which matches the descriptions in LLVM). Since the clang option is thin-link-bitcode, I went with that to try and not add a new spelling in the world. Per @dtolnay, you can work around the lack of this by using `lld --thinlto-index-only` to do the indexing on regular .o files of bitcode, but that is a bit wasteful on actions when we already have all the information in rustc and could just write out the matching minimized bitcode. I didn't test that at all in our infrastructure, because by the time I learned that I already had this patch largely written.
This was needed in an older version of this patch, but never got edited out when it became obsolete.
If we don't do this, some versions of LLVM (at least 17, experimentally) will double-emit some error messages, which is how I noticed this. Given that it seems to be costing some extra work, let's only request the summary bitcode production if we'll actually bother writing it down, otherwise skip it.
I did this in the user-facing logic, but I noticed while fixing a minor defect that I had missed it in a few places in the internal details.
rust-lld: fallback to rustc's sysroot if there's no path to the linker in the target sysroot As seen in rust-lang#125246, some sysroots don't expect to contain `rust-lld` and want to keep it that way, so we fallback to the default rustc sysroot if there is no path to the linker in any of the sysroot tools search paths. This is how we locate codegen-backends' dylibs already. People also have requested an error if none of these search paths contain the self-contained linker directory, so there's also an error in that case. r? `@petrochenkov` cc `@ehuss` `@RalfJung` I'm not sure where we check for `rust-lld`'s existence on the targets where we use it by default, and if we just ignore it when missing or emit a warning (as I assume we don't emit an error), so I just checked for the existence of `gcc-ld`, where `cc` will look for the lld-wrapper binaries. <sub>*Feel free to point out better ways to do this, it's the middle of the night here.*</sub> Fixes rust-lang#125246
rustc_codegen_llvm: add support for writing summary bitcode Typical uses of ThinLTO don't have any use for this as a standalone file, but distributed ThinLTO uses this to make the linker phase more efficient. With clang you'd do something like `clang -flto=thin -fthin-link-bitcode=foo.indexing.o -c foo.c` and then get both foo.o (full of bitcode) and foo.indexing.o (just the summary or index part of the bitcode). That's then usable by a two-stage linking process that's more friendly to distributed build systems like bazel, which is why I'm working on this area. I talked some to `@teresajohnson` about naming in this area, as things seem to be a little confused between various blog posts and build systems. "bitcode index" and "bitcode summary" tend to be a little too ambiguous, and she tends to use "thin link bitcode" and "minimized bitcode" (which matches the descriptions in LLVM). Since the clang option is thin-link-bitcode, I went with that to try and not add a new spelling in the world. Per `@dtolnay,` you can work around the lack of this by using `lld --thinlto-index-only` to do the indexing on regular .o files of bitcode, but that is a bit wasteful on actions when we already have all the information in rustc and could just write out the matching minimized bitcode. I didn't test that at all in our infrastructure, because by the time I learned that I already had this patch largely written.
Actually use TAIT instead of emulating it `core`'s `impl_fn_for_zst` macro is just a hacky way of emulating TAIT. TAIT has become stable enough to be used [in other places](https://github.com/rust-lang/rust/blob/e8fbd991287f637f95016a71ddc13438415bbe59/library/std/src/backtrace.rs#L431) inside the standard library, so let's use it in `core` as well.
…esleywiser Don't suggest adding the unexpected cfgs to the build-script it-self This PR adds a check to avoid suggesting to add the unexpected cfgs inside the build-script when building the build-script it-self, as it won't have any effect, since build-scripts applies to their descended target. Fixes rust-lang#125368
…rt-out-dir, r=jieyouxu Migrate `run-make/rustdoc-with-short-out-dir-option` to `rmake.rs` Part of rust-lang#121876. r? `@jieyouxu`
… r=bjorn3 Cleanup check-cfg handling in core and std Follow-up to rust-lang#125296 where we: - expect any feature cfg in std, due to `#[path]` imports - move some check-cfg args inside the `build.rs` as per Cargo recommendation - and replace the fake Cargo feature `"restricted-std"` by the custom cfg `restricted_std` Fixes rust-lang#125296 (comment) r? `@bjorn3` (maybe, feel free to re-roll)
rustbot
added
A-testsuite
|
@bors r+ p=5 rollup=never |
bors
added
S-waiting-on-bors
…llaumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#125263 (rust-lld: fallback to rustc's sysroot if there's no path to the linker in the target sysroot) - rust-lang#125345 (rustc_codegen_llvm: add support for writing summary bitcode) - rust-lang#125362 (Actually use TAIT instead of emulating it) - rust-lang#125412 (Don't suggest adding the unexpected cfgs to the build-script it-self) - rust-lang#125445 (Migrate `run-make/rustdoc-with-short-out-dir-option` to `rmake.rs`) - rust-lang#125452 (Cleanup check-cfg handling in core and std) r? `@ghost` `@rustbot` modify labels: rollup
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
spurious |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR:
previous master: 78dd504f2f In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
|
Finished benchmarking commit (7601adc): comparison URL. Overall result: ❌ regressions - 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)Results (primary -3.2%, secondary 2.7%)This 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.
CyclesResults (secondary 7.1%)This 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.
Binary sizeResults (primary -0.1%)This 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: 672.268s -> 673.634s (0.20%) |
|
It’s probably #125263 where I do more work upfront to verify the sysroot is actually complete and contains the self-contained linker and emit an error otherwise. If that’s the case it’s fine, and helloworld could be now fast enough that checking if 2 dirs exist is a noticeable regression. We also wanted to embellish gcc’s awful error a posteriori so we could also move some of that work there. @rust-timer build 746de61 |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (746de61): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDInstruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results (secondary 2.3%)This 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.
CyclesResults (secondary 2.0%)This 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.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 672.268s -> 673.052s (0.12%) |
|
The cachegrind diff is the very helpful I may try to keep the incomplete-sysroot fallback paths, but only check for their existence if linking with lld failed, just to see if it changes anything compared to the tiny new allocations/sysroot detections. But otherwise this is fine, it's basically only in icounts and people want an error in these cases. @rustbot label: +perf-regression-triaged |
rustbot
added
the
perf-regression-triaged
Successful merges:
run-make/rustdoc-with-short-out-dir-optiontormake.rs#125445 (Migraterun-make/rustdoc-with-short-out-dir-optiontormake.rs)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup