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 11 pull requests #79167

Merged
merged 28 commits into from
Nov 18, 2020
Merged

Rollup of 11 pull requests #79167

merged 28 commits into from
Nov 18, 2020

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Nov 18, 2020

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

DevJPM and others added 28 commits October 25, 2020 17:06
This PR both adds in-source documentation on what to look out for
when adding a new (X86) feature set and adds all that are detectable at run-time in Rust stable
as of 1.27.0.

This should only enable the use of the corresponding LLVM intrinsics.
Actual intrinsics need to be added separately in rust-lang/stdarch.

It also re-orders the run-time-detect test statements to be more consistent
with the actual list of intrinsics whitelisted and removes underscores not present
in the actual names (which might be mistaken as being part of the name)
`movbe` seems to not be a run-time detectable feature on x86.
It has thus been removed from the list.
It was only commented out to ease comparison against the full list.
…M 9 exclusive features

Updated the added documentation in llvm_util.rs to note which copies of LLVM need to be inspected.
Removed avx512bf16 and avx512vp2intersect because they are unsupported before LLVM 9 with the build with external LLVM 8 being supported
Re-introduced detection testing previously removed for un-requestable features tsc and mmx
With rust-lang#78848 merged, the minimum supported LLVM version is now 9
which means we can actually use the target features introduced in LLVM 9
We'll try to use a weak `getrandom` symbol first, because that allows
things like `LD_PRELOAD` interposition. For example, perf measurements
might want to disable randomness to get reproducible results. If the
weak symbol is not found, we fall back to a raw `SYS_getrandom` call.
Historically the stable tarballs were named after the version number of
the specific tool, instead of the version number of Rust. For example,
both of the following tarballs were part of the same release:

    rustc-1.48.0-x86_64-unknown-linux-gnu.tar.xz
    cargo-0.49.0-x86_64-unknown-linux-gnu.tar.xz

PR rust-lang#77336 changed the dist code to instead use Rust's version number for
all the tarballs, regardless of the tool they contain:

    rustc-1.48.0-x86_64-unknown-linux-gnu.tar.xz
    cargo-1.48.0-x86_64-unknown-linux-gnu.tar.xz

Because of that there is no need anymore to have a separate `cargo`
field in src/stage0.txt, as the Cargo version will always be the same as
the rustc version. This PR removes the field, simplifying the code and
the maintenance work required while producing releases.
This should make Clippy more resilient and will unblock rust-lang#78343.

This PR is made against rust-lang/rust to avoid the need for a subtree
sync at @flip1995's suggestion in rust-lang/rust-clippy#6310.
These referred to a “`Write`er”—extra *e*. Presumably a copy-paste
holdover from “`Read`er”.

Test Plan:
Running ``git grep '`\?[Ww]rite`\?er'`` no longer finds any results.

wchargin-branch: io-write-docs
Updated the list of white-listed target features for x86

This PR both adds in-source documentation on what to look out for when adding a new (X86) feature set and [adds all that are detectable at run-time in Rust stable as of 1.27.0](https://github.com/rust-lang/stdarch/blob/master/crates/std_detect/src/detect/arch/x86.rs).

This should only enable the use of the corresponding LLVM intrinsics.
Actual intrinsics need to be added separately in rust-lang/stdarch.

It also re-orders the run-time-detect test statements to be more consistent
with the actual list of intrinsics whitelisted and removes underscores not present
in the actual names (which might be mistaken as being part of the name)

The reference for LLVM's feature names used is [this file](https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/X86TargetParser.def).

This PR was motivated as the compiler end's part for allowing rust-lang#67329 to be adressed over on rust-lang/stdarch
linux: try to use libc getrandom to allow interposition

We'll try to use a weak `getrandom` symbol first, because that allows
things like `LD_PRELOAD` interposition. For example, perf measurements
might want to disable randomness to get reproducible results. If the
weak symbol is not found, we fall back to a raw `SYS_getrandom` call.
stability: More precise location for deprecation lint on macros

One missing piece of rust-lang#73178.
Tighten the bounds on atomic Ordering in std::sys::unix::weak::Weak

This moves reading this from multiple SeqCst reads to Relaxed read + Acquire fence if we are actually going to use the data.

Would love to avoid the Acquire fence, but doing so would need Ordering::Consume, which neither Rust, nor LLVM supports (a shame, since this fence is hardly free on ARM, which is what I was hoping to improve).

r? ``@Amanieu`` (Sorry for always picking you, but I know a lot of people wouldn't feel comfortable reviewing atomic ordering changes)
…sper

Turn top-level comments into module docs in MIR visitor
…eros, r=m-ou-se

add trailing_zeros and leading_zeros to non zero types

as a way towards being able to use the optimized intrinsics ctlz_nonzero and cttz_nonzero from stable.

have not crated any tracking issue if this is not a solution that is wanted
Enable AVX512 *epi64 variants by updating stdarch
…k-Simulacrum

bootstrap: use the same version number for rustc and cargo

Historically the stable tarballs were named after the version number ofthe specific tool, instead of the version number of Rust. For example, both of the following tarballs were part of the same release:

    rustc-1.48.0-x86_64-unknown-linux-gnu.tar.xz
    cargo-0.49.0-x86_64-unknown-linux-gnu.tar.xz

PR rust-lang#77336 changed the dist code to instead use Rust's version number for all the tarballs, regardless of the tool they contain:

    rustc-1.48.0-x86_64-unknown-linux-gnu.tar.xz
    cargo-1.48.0-x86_64-unknown-linux-gnu.tar.xz

Because of that there is no need anymore to have a separate `cargo` field in `src/stage0.txt`, as the Cargo version will always be the same as the rustc version. This PR removes the field, simplifying the code and the maintenance work required while producing releases.

r? ``@Mark-Simulacrum``
Fix handling of panic calls

This should make Clippy more resilient and will unblock rust-lang#78343.

This PR is made against rust-lang/rust to avoid the need for a subtree
sync at ``@flip1995's`` suggestion in rust-lang/rust-clippy#6310.

r? ``@flip1995``
cc ``@m-ou-se``
…jyn514

Fix typo in `std::io::Write` docs

These referred to a “`Write`er”—extra *e*. Presumably a copy-paste
holdover from “`Read`er”.

Test Plan:
Running ``git grep '`\?[Ww]rite`\?er'`` no longer finds any results.

wchargin-branch: io-write-docs
type is too big -> values of the type are too big

strictly speaking, `[u8; usize::MAX]` or even `[[[u128; usize::MAX]; usize::MAX]; usize::MAX]` are absolutely fine types as long as you don't try to deal with any values of it.

This error message seems to cause some confusion imo, for example in rust-lang#79135 (comment) so I would prefer us to be more precise here.

See the added test case which uses one of these types without causing an error.

r? ``@oli-obk``
@rustbot rustbot added the rollup A PR which is a rollup label Nov 18, 2020
@m-ou-se
Copy link
Member Author

m-ou-se commented Nov 18, 2020

@bors r+ p=11 rollup=never

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2020
@bors
Copy link
Contributor

bors commented Nov 18, 2020

⌛ Testing commit 43d13e2 with merge 8d2d001...

@bors
Copy link
Contributor

bors commented Nov 18, 2020

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 8d2d001 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 18, 2020
@bors bors merged commit 8d2d001 into rust-lang:master Nov 18, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 18, 2020
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #79167!

Tested on commit 8d2d001.
Direct link to PR: #79167

💔 miri on linux: test-pass → test-fail (cc @oli-obk @eddyb @RalfJung).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Nov 18, 2020
Tested on commit rust-lang/rust@8d2d001.
Direct link to PR: <rust-lang/rust#79167>

💔 miri on linux: test-pass → test-fail (cc @oli-obk @eddyb @RalfJung).
@m-ou-se m-ou-se deleted the rollup-4g15apk branch November 18, 2020 21:27
@jyn514
Copy link
Member

jyn514 commented Nov 24, 2020

This had up to 7.7% regressions on instruction counts and a 40% increase in the time to bootstrap libcore. I suspect #78785 might be to blame. @cuviper did you expect a performance regression for that PR? If not, does it make sense to back it out and try to land a modified version that doesn't affect perf as badly?

@cuviper
Copy link
Member

cuviper commented Nov 24, 2020

I didn't expect a measurable impact, but it's possible the weak symbols have more effect than I realized. #79039 also modified the atomic ordering in that area. Maybe someone can git bisect local builds to drill into this roll-up?

@m-ou-se
Copy link
Member Author

m-ou-se commented Nov 24, 2020

Interesting, those are huge differences. None of these PRs seem suspicious though, at first glance.

The weak syscall lookup should only happen once. Any later calls just end up doing a single atomic operation, which is completely insignificant compared to the syscall they execute right after. It'd be quite surprising if the first lookup has such a big effect.

@jyn514
Copy link
Member

jyn514 commented Nov 24, 2020

Hmm, most of the change is in metadata_decode_entry_item_children. Maybe the increase in doc-comments or new stdarch module affected it somehow?

@m-ou-se
Copy link
Member Author

m-ou-se commented Nov 24, 2020

The stdarch change seems to add quite a lot of things:

image

@jyn514
Copy link
Member

jyn514 commented Nov 25, 2020

@m-ou-se @cuviper you were right, #78785 had almost no impact. #79131 definitely caused the bootstrap increase but strangely instruction counts went down with that PR reverted. So I'm still stumped what caused the 7% regression on deeply-nested-async-opt.

@Mark-Simulacrum
Copy link
Member

Yeah both reverts didn't obviously affect it at all (not even the 2-3% regressions on non-incr bits), but that benchmark is also a bit noisy historically so I wouldn't worry too much about it.

@RalfJung
Copy link
Member

but strangely instruction counts went down with that PR reverted

Isn't that what you'd expect from reverting a slowdown PR?

@tgnottingham
Copy link
Contributor

This rollup also introduced an increase in max-rss across the board. And it looks like #79131 was responsible, based on the corresponding max-rss decrease in the revert PR #79391.

By the way, the 7% instruction count regression on deeply-nested-async is likely a red herring. It sometimes varies on its own, seemingly without reason (see here and here). It's probably prone to variance depending on CGU partitioning. And part of the difference was due to #79131.

It looks to me like #79131 was responsible for all or nearly all of the full build instruction count regressions, and part of the incremental build instruction count regressions. The rest of the incremental regressions are unaccounted for, but they're fairly small, once you account for the effect of #79131.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet