| Age | Commit message (Collapse) | Author | Lines |
|---|
|
Fix cases where std accidentally relied on inline(never)
This PR increases the power of `-Zcross-crate-inline-threshold=always` so that it applies through `#[inline(never)]`. Note that though this is called "cross-crate-inlining" in this case especially it is _just_ lazy per-CGU codegen. The MIR inliner and LLVM still respect the attribute as much as they ever have.
Trying to bootstrap with the new `-Zcross-crate-inline-threshold=always` change revealed two bugs:
We have special intrinsics `assert_inhabited`, `assert_zero_valid`, and `assert_mem_uniniitalized_valid` which codegen backends will lower to nothing or a call to `panic_nounwind`. Since we may not have any call to `panic_nounwind` in MIR but emit one anyway, we need to specially tell `MirUsedCollector` about this situation.
`#[lang = "start"]` is special-cased already so that `MirUsedCollector` will collect it, but then when we make it cross-crate-inlinable it is only assigned to a CGU based on whether `MirUsedCollector` saw a call to it, which of course we didn't.
---
I started looking into this because https://github.com/rust-lang/rust/pull/118683 revealed a case where we were accidentally relying on a function being `#[inline(never)]`, and cranking up cross-crate-inlinability seems like a way to find other situations like that.
r? `@nnethercote` because I don't like what I'm doing to the CGU partitioning code here but I can't come up with something much better
|
|
|
|
The new flag has been described in the Major Change Proposal at
https://github.com/rust-lang/compiler-team/issues/656
|
|
`x clippy compiler -Aclippy::all -Wclippy::needless_borrow --fix`.
Then I had to remove a few unnecessary parens and muts that were exposed
now.
|
|
This query has a name that sounds general-purpose, but in fact it has
coverage-specific semantics, and (fortunately) is only used by coverage code.
Because it is only ever called once (from one designated CGU), it doesn't need
to be a query, and we can change it to a regular function instead.
|
|
|
|
This fixes the changes brought to codegen tests when effect params are
added to libcore, by not attempting to monomorphize functions that get
the host param by being `const fn`.
|
|
Tweak CGU sorting in a couple of places.
In `base.rs`, tweak how the CGU size interleaving works. Since #113777, it's much more common to have multiple CGUs with identical sizes. With the existing code these same-sized items ended up in the opposite-to-desired order due to the stable sorting. The code now starts with a reverse sort (like is done in `partitioning.rs`) which gives the behaviour we want. This doesn't matter much for perf, but makes profiles in `samply` look more like what we expect.
In `partitioning.rs`, we can use `sort_by_key` instead of `sort_by_cached_key` because `CGU::size_estimate()` is cheap. (There is an identical CGU sort earlier in that function that already uses `sort_by_key`.)
r? `@pnkfelix`
|
|
get_first
single_char_add_str
unnecessary_mut_passed
manual_map
manual_is_ascii_check
|
|
comparison_to_empty
iter_nth_zero
for_kv_map
manual_next_back
redundant_pattern
|
|
In `base.rs`, tweak how the CGU size interleaving works. Since #113777,
it's much more common to have multiple CGUs with identical sizes. With
the existing code these same-sized items ended up in the
opposite-to-desired order due to the stable sorting. The code now starts
with a reverse sort (like is done in `partitioning.rs`) which gives the
behaviour we want. This doesn't matter much for perf, but makes profiles
in `samply` look more like what we expect.
In `partitioning.rs`, we can use `sort_by_key` instead of
`sort_by_cached_key` because `CGU::size_estimate()` is cheap. (There is
an identical CGU sort earlier in that function that already uses
`sort_by_key`.)
|
|
Instead of repeatedly merging the two smallest CGUs, we now use a
merging algorithm that aims to minimize the duplication of inlined
functions.
`exa-0.10.1` was one benchmark that saw particularly good results. The
old CGU stats:
```
INTERNALIZE
- unique items: 2774 (1216 root + 1558 inlined), unique size: 122065 (77219 root + 44846 inlined)
- placed items: 3834 (1216 root + 2618 inlined), placed size: 154552 (77219 root + 77333 inlined)
- placed/unique items ratio: 1.38, placed/unique size ratio: 1.27
- CGUs: 16, mean size: 9659.5, sizes: [11791, 11634, 11173, 10987, 10939, 10507, 9992, 9813, 9593, 9580, 9030, 8447, 7975, 7961, 7876, 7254]
```
The new CGU stats:
```
INTERNALIZE
- unique items: 2774 (1216 root + 1558 inlined), unique size: 122065 (77219 root + 44846 inlined)
- placed items: 3626 (1216 root + 2410 inlined), placed size: 147201 (77219 root + 69982 inlined)
- placed/unique items ratio: 1.31, placed/unique size ratio: 1.21
- CGUs: 16, mean size: 9200.1, sizes: [11634, 10939, 10227, 9555, 9178, 9167, 8879, 8804, 8604, 8603 (x3), 8602 (x2), 8601, 8600]
```
The difference is in the number of inlined items. There are 1558 unique
inlined items. With the old algorithm these were placed 2618 times,
resulting in 1060 duplicates. With the new algorithm these were placed
2410 times, resulting in 852 duplicates. Also, the mean CGU size dropped
from 9659.5 to 9200.1, and the CGU size distribution tightened, with the
biggest one a little smaller and the smallest ones a little bigger.
|
|
It has two conditions. This commit splits it in two, one per condition.
The next commit will change the first loop.
|
|
|
|
They're quite rare, and ignoring them simplifies things quite a bit, and
further reduces the number of calls to `MonoItem::size_estimate` to the
number of placed items (one per root item, and one or more per reachable
inlined item).
|
|
This means we call `MonoItem::size_estimate` (which involves a query)
less often: just once per mono item, and then once more per inline item
placement. After that we can reuse the stored value as necessary. This
means `CodegenUnit::compute_size_estimate` is cheaper.
|
|
It replaces `(Linkage, Visibility)`, making the code nicer. Plus the
next commit will add another field.
|
|
|
|
CGU formation tweaks
Minor improvements I found while trying out something bigger that didn't work out.
r? ``@wesleywiser``
|
|
An assertion failure was reported in #112946. This extra information
will help diagnose the problem.
|
|
|
|
It's needless verbosity.
|
|
|
|
It's nicer this way.
|
|
For non-incremental builds on Unix, currently all the thread names look
like `opt regex.f10ba03eb5ec7975-cgu.0`. But they are truncated by
`pthread_setname` to `opt regex.f10ba`, hiding the numeric suffix that
distinguishes them. This is really annoying when using a profiler like
Samply.
This commit changes these thread names to a form like `opt cgu.0`, which
is much better.
|
|
Currently there are two problems.
First, the CGUS don't end up in size order. The merging loop does sort
by size on each iteration, but we don't sort after the final merge, so
typically there is one CGU out of place. (And sometimes we don't enter
the merging loop at all, in which case they end up in random order.)
Second, we then assign names that differ only by a numeric suffix, and
then we sort them lexicographically by name, giving us an order like
this:
regex.f10ba03eb5ec7975-cgu.1
regex.f10ba03eb5ec7975-cgu.10
regex.f10ba03eb5ec7975-cgu.11
regex.f10ba03eb5ec7975-cgu.12
regex.f10ba03eb5ec7975-cgu.13
regex.f10ba03eb5ec7975-cgu.14
regex.f10ba03eb5ec7975-cgu.15
regex.f10ba03eb5ec7975-cgu.2
regex.f10ba03eb5ec7975-cgu.3
regex.f10ba03eb5ec7975-cgu.4
regex.f10ba03eb5ec7975-cgu.5
regex.f10ba03eb5ec7975-cgu.6
regex.f10ba03eb5ec7975-cgu.7
regex.f10ba03eb5ec7975-cgu.8
regex.f10ba03eb5ec7975-cgu.9
These two problems are really annoying when debugging and profiling the
CGUs.
This commit ensures CGUs are sorted by name *and* reverse sorted by
size. This involves (a) one extra sort by size operation, and (b)
padding the numeric indices with zeroes, e.g.
`regex.f10ba03eb5ec7975-cgu.01`.
(Note that none of this applies for incremental builds, where a
different hash-based CGU naming scheme is used.)
|
|
- Rename `create_size_estimate` as `compute_size_estimate`, because that
makes more sense for the second and subsequent calls for each CGU.
- Change `CodegenUnit::size_estimate` from `Option<usize>` to `usize`.
We can still assert that `compute_size_estimate` is called first.
- Move the size estimation for `place_mono_items` inside the function,
for consistency with `merge_codegen_units`.
|
|
There's no longer any need for them to be separate, and putting them
together reduces the amount of code.
|
|
Because CGU merging relies on CGU sizes, but the CGU sizes before
inlining aren't accurate.
This requires tweaking how the sizes are updated during merging: if CGU
A and B both have an inlined function F, then `size(A + B)` will be a
little less than `size(A) + size(B)`, because `A + B` will only have one
copy of F. Also, the minimum CGU size is increased because it now has to
account for inlined functions.
This change doesn't have much effect on compile perf, but it makes
follow-on changes that involve more sophisticated reasoning about CGU
sizes much easier.
|
|
|
|
|
|
Always put the `create_size_estimate` calls and `debug_dump` calls
within a timed scopes. This makes the four main steps look more similar
to each other.
|
|
The comment says "Find the smallest CGU that has exported symbols and
put the dead function stubs in that CGU". But the code sorts the CGUs by
size (smallest first) and then searches them in reverse order, which
means it will find the *largest* CGU that has exported symbols.
The erroneous code was introduced in #92142.
This commit changes it to use a simpler search, avoiding the sort, and
fixes the bug in the process.
|
|
The other major steps in `partition` have their own function, so it's
nice for this one to be likewise.
|
|
Because tiny CGUs make compilation less efficient *and* result in worse
generated code.
We don't do this when the number of CGUs is explicitly given, because
there are times when the requested number is very important, as
described in some comments within the commit. So the commit also
introduces a `CodegenUnits` type that distinguishes between default
values and user-specified values.
This change has a roughly neutral effect on walltimes across the
rustc-perf benchmarks; there are some speedups and some slowdowns. But
it has significant wins for most other metrics on numerous benchmarks,
including instruction counts, cycles, binary size, and max-rss. It also
reduces parallelism, which is good for reducing jobserver competition
when multiple rustc processes are running at the same time. It's smaller
benchmarks that benefit the most; larger benchmarks already have CGUs
that are all larger than the minimum size.
Here are some example before/after CGU sizes for opt builds.
- html5ever
- CGUs: 16, mean size: 1196.1, sizes: [3908, 2992, 1706, 1652, 1572,
1136, 1045, 948, 946, 938, 579, 471, 443, 327, 286, 189]
- CGUs: 4, mean size: 4396.0, sizes: [6706, 3908, 3490, 3480]
- libc
- CGUs: 12, mean size: 35.3, sizes: [163, 93, 58, 53, 37, 8, 2 (x6)]
- CGUs: 1, mean size: 424.0, sizes: [424]
- tt-muncher
- CGUs: 5, mean size: 1819.4, sizes: [8508, 350, 198, 34, 7]
- CGUs: 1, mean size: 9075.0, sizes: [9075]
Note that CGUs of size 100,000+ aren't unusual in larger programs.
|
|
For example, we go from this:
```
FINAL (4059 items, total_size=232342; 16 CGUs, max_size=39608,
min_size=5468, max_size/min_size=7.2):
- CGU[0] regex.f2ff11e98f8b05c7-cgu.0 (318 items, size=39608):
- fn ...
- fn ...
```
to this:
```
FINAL
- unique items: 2726 (1459 root + 1267 inlined), unique size: 201214 (146046 root + 55168 inlined)
- placed items: 4059 (1459 root + 2600 inlined), placed size: 232342 (146046 root + 86296 inlined)
- placed/unique items ratio: 1.49, placed/unique size ratio: 1.15
- CGUs: 16, mean size: 14521.4, sizes: [39608, 31122, 20318, 20236, 16268, 13777, 12310, 10531, 10205, 9810, 9250, 9065 (x2), 7785, 7524, 5468]
- CGU[0]
- regex.f2ff11e98f8b05c7-cgu.0, size: 39608
- items: 318, mean size: 124.6, sizes: [28395, 3418, 558, 485, 259, 228, 176, 166, 146, 118, 117 (x3), 114 (x5), 113 (x3), 101, 84, 82, 77, 76, 72, 71 (x2), 66, 65, 62, 61, 59 (x2), 57, 55, 54 (x2), 53 (x4), 52 (x5), 51 (x4), 50, 48, 47, 46, 45 (x3), 44, 43 (x5), 42, 40, 38 (x4), 37, 35, 34 (x2), 32 (x2), 31, 30, 28 (x2), 27 (x2), 26 (x3), 24 (x2), 23 (x3), 22 (x2), 21, 20, 16 (x4), 15, 13 (x7), 12 (x3), 11 (x6), 10, 9 (x2), 8 (x4), 7 (x8), 6 (x38), 5 (x21), 4 (x7), 3 (x45), 2 (x63), 1 (x13)]
- fn ...
- fn ...
```
This is a lot more information, distinguishing between root items and
inlined items, showing how much duplication there is of inlined items,
plus the full range of sizes for CGUs and items within CGUs. All of
which is really helpful when analyzing this stuff and trying different
CGU formation algorithms.
|
|
Because that value can be easily obtained from `Partitioning::tcx`.
|
|
It's currently created in `place_inlined_mono_items` and then used in
`internalize_symbols`. This commit moves the creation to
`internalize_symbols`.
|
|
It's no longer used.
|
|
This loop is doing two different things. For inlined items, it's adding
them to the CGU. For all items, it's recording them in
`mono_item_placements`.
This commit splits it into two separate loops. This avoids putting root
mono items into `reachable`, and removes the low-value check that
`roots` doesn't contain inlined mono items.
|
|
Because they have a lot of overlap.
|
|
Because the next commit will merge them.
|
|
Currently it sorts by symbol name, which is a mangled name like
`_ZN1a4main17hb29587cdb6db5f42E`, which leads to non-obvious orderings.
This commit changes it to use the existing
`items_in_deterministic_order`, which iterates in source code order.
|
|
|
|
I found this confusing because it includes the root item, plus the
inlined items reachable from the root item. The new formulation
separates the two parts more clearly.
|
|
Currently it overwrites all the CGUs with new CGUs. But those new CGUs
are just copies of the old CGUs, possibly with some things added. This
commit changes things so that each CGU just gets added to in place,
which makes things simpler and clearer.
|
|
`UsageMap` contains `used_map`, which maps from an item to the item it
uses. This commit add `user_map`, which is the inverse.
We already compute this inverse, but later on, and it is only held as a
local variable. Its simpler and nicer to put it next to `used_map`.
|
|
Currently, the code uses multiple words to describe when a mono item `f`
uses a mono item `g`, all of which have problems.
- `f` references `g`: confusing because there are multiple kinds of use,
e.g. "`f` calls `g`" is one, but "`f` takes a (`&T`-style) reference
of `g`" is another, and that's two subtly different meanings of
"reference" in play.
- `f` accesses `g`: meh, "accesses" makes me think of data, and this is
code.
- `g` is a neighbor (or neighbour) of `f`: is verbose, and doesn't
capture the directionality.
This commit changes the code to use "`f` uses `g`" everywhere. I think
it's better than the current terminology, and the consistency is
important.
Also, `InliningMap` is renamed `UsageMap` because (a) it was always
mostly about usage, and (b) the inlining information it did record was
removed in a recent commit.
|
|
Improve CGU debug printing.
- Add more total and per-CGU measurements.
- Ensure CGUs are sorted by name before the first `debug_dump` calls, for deterministic output.
- Print items within CGUs in sorted-by-name order, for deterministic output.
- Add some assertions and comments clarifying sortedness of CGUs at various points.
An example, before:
```
INITIAL PARTITIONING (5 CodegenUnits, max=29, min=1, max/min=29.0):
CodegenUnit scev95ysd7g4b0z estimated size 2:
- fn <() as std::process::Termination>::report [(External, Hidden)] [h082b15a6d07338dcE] estimated size 2
CodegenUnit 1j0frgtl72rsz24q estimated size 29:
- fn std::rt::lang_start::<()>::{closure#0} [(External, Hidden)] [h695c7b5d6a212565E] estimated size 17
- fn std::rt::lang_start::<()> [(External, Hidden)] [h4ca942948e9cb931E] estimated size 12
CodegenUnit 5dbzi1e5qm0d7kj2 estimated size 4:
- fn <[closure@std::rt::lang_start<()>::{closure#0}] as std::ops::FnOnce<()>>::call_once - shim [(External, Hidden)] [h24eaa44f03b2b233E] estimated size 1
- fn <fn() as std::ops::FnOnce<()>>::call_once - shim(fn()) [(External, Hidden)] [hf338f5339c3711acE] estimated size 1
- fn <[closure@std::rt::lang_start<()>::{closure#0}] as std::ops::FnOnce<()>>::call_once - shim(vtable) [(External, Hidden)] [h595d414cbb7651d5E] estimated size 1
- fn std::ptr::drop_in_place::<[closure@std::rt::lang_start<()>::{closure#0}]> - shim(None) [(External, Hidden)] [h17a19dcdb40600daE] estimated size 1
CodegenUnit 220m1mqa2mlbg7r3 estimated size 1:
- fn main [(External, Hidden)] [hb29587cdb6db5f42E] estimated size 1
CodegenUnit 4ulbh241f7tvyn7x estimated size 6:
- fn std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()> [(External, Hidden)] [h41dada2c21a1259dE] estimated size 6
```
and after:
```
INITIAL PARTITIONING (9 items, total_size=42; 5 CGUs, max_size=29, min_size=1, max_size/min_size=29.0):
- CGU[0] 1j0frgtl72rsz24q (2 items, size=29):
- fn std::rt::lang_start::<()> [(External, Hidden)] [h4ca942948e9cb931E] (size=12)
- fn std::rt::lang_start::<()>::{closure#0} [(External, Hidden)] [h695c7b5d6a212565E] (size=17)
- CGU[1] 220m1mqa2mlbg7r3 (1 items, size=1):
- fn main [(External, Hidden)] [hb29587cdb6db5f42E] (size=1)
- CGU[2] 4ulbh241f7tvyn7x (1 items, size=6):
- fn std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()> [(External, Hidden)] [h41dada2c21a1259dE] (size=6)
- CGU[3] 5dbzi1e5qm0d7kj2 (4 items, size=4):
- fn <[closure@std::rt::lang_start<()>::{closure#0}] as std::ops::FnOnce<()>>::call_once - shim(vtable) [(External, Hidden)] [h595d414cbb7651d5E] (size=1)
- fn <[closure@std::rt::lang_start<()>::{closure#0}] as std::ops::FnOnce<()>>::call_once - shim [(External, Hidden)] [h24eaa44f03b2b233E] (size=1)
- fn <fn() as std::ops::FnOnce<()>>::call_once - shim(fn()) [(External, Hidden)] [hf338f5339c3711acE] (size=1)
- fn std::ptr::drop_in_place::<[closure@std::rt::lang_start<()>::{closure#0}]> - shim(None) [(External, Hidden)] [h17a19dcdb40600daE] (size=1)
- CGU[4] scev95ysd7g4b0z (1 items, size=2):
- fn <() as std::process::Termination>::report [(External, Hidden)] [h082b15a6d07338dcE] (size=2)
```
r? ``@wesleywiser``
|
|
- Add more total and per-CGU measurements.
- Ensure CGUs are sorted by name before the first `debug_dump` calls,
for deterministic output.
- Print items within CGUs in sorted-by-name order, for deterministic
output.
- Add some assertions and comments clarifying sortedness of CGUs at
various points.
An example, before:
```
INITIAL PARTITIONING (5 CodegenUnits, max=29, min=1, max/min=29.0):
CodegenUnit scev95ysd7g4b0z estimated size 2:
- fn <() as std::process::Termination>::report [(External, Hidden)] [h082b15a6d07338dcE] estimated size 2
CodegenUnit 1j0frgtl72rsz24q estimated size 29:
- fn std::rt::lang_start::<()>::{closure#0} [(External, Hidden)] [h695c7b5d6a212565E] estimated size 17
- fn std::rt::lang_start::<()> [(External, Hidden)] [h4ca942948e9cb931E] estimated size 12
CodegenUnit 5dbzi1e5qm0d7kj2 estimated size 4:
- fn <[closure@std::rt::lang_start<()>::{closure#0}] as std::ops::FnOnce<()>>::call_once - shim [(External, Hidden)] [h24eaa44f03b2b233E] estimated size 1
- fn <fn() as std::ops::FnOnce<()>>::call_once - shim(fn()) [(External, Hidden)] [hf338f5339c3711acE] estimated size 1
- fn <[closure@std::rt::lang_start<()>::{closure#0}] as std::ops::FnOnce<()>>::call_once - shim(vtable) [(External, Hidden)] [h595d414cbb7651d5E] estimated size 1
- fn std::ptr::drop_in_place::<[closure@std::rt::lang_start<()>::{closure#0}]> - shim(None) [(External, Hidden)] [h17a19dcdb40600daE] estimated size 1
CodegenUnit 220m1mqa2mlbg7r3 estimated size 1:
- fn main [(External, Hidden)] [hb29587cdb6db5f42E] estimated size 1
CodegenUnit 4ulbh241f7tvyn7x estimated size 6:
- fn std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()> [(External, Hidden)] [h41dada2c21a1259dE] estimated size 6
```
and after:
```
INITIAL PARTITIONING (9 items, total_size=42; 5 CGUs, max_size=29, min_size=1, max_size/min_size=29.0):
- CGU[0] 1j0frgtl72rsz24q (2 items, size=29):
- fn std::rt::lang_start::<()> [(External, Hidden)] [h4ca942948e9cb931E] (size=12)
- fn std::rt::lang_start::<()>::{closure#0} [(External, Hidden)] [h695c7b5d6a212565E] (size=17)
- CGU[1] 220m1mqa2mlbg7r3 (1 items, size=1):
- fn main [(External, Hidden)] [hb29587cdb6db5f42E] (size=1)
- CGU[2] 4ulbh241f7tvyn7x (1 items, size=6):
- fn std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()> [(External, Hidden)] [h41dada2c21a1259dE] (size=6)
- CGU[3] 5dbzi1e5qm0d7kj2 (4 items, size=4):
- fn <[closure@std::rt::lang_start<()>::{closure#0}] as std::ops::FnOnce<()>>::call_once - shim(vtable) [(External, Hidden)] [h595d414cbb7651d5E] (size=1)
- fn <[closure@std::rt::lang_start<()>::{closure#0}] as std::ops::FnOnce<()>>::call_once - shim [(External, Hidden)] [h24eaa44f03b2b233E] (size=1)
- fn <fn() as std::ops::FnOnce<()>>::call_once - shim(fn()) [(External, Hidden)] [hf338f5339c3711acE] (size=1)
- fn std::ptr::drop_in_place::<[closure@std::rt::lang_start<()>::{closure#0}]> - shim(None) [(External, Hidden)] [h17a19dcdb40600daE] (size=1)
- CGU[4] scev95ysd7g4b0z (1 items, size=2):
- fn <() as std::process::Termination>::report [(External, Hidden)] [h082b15a6d07338dcE] (size=2)
```
|
