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 10 pull requests #73504

Merged
merged 47 commits into from
Jun 19, 2020
Merged

Rollup of 10 pull requests #73504

merged 47 commits into from
Jun 19, 2020

Conversation

RalfJung
Copy link
Member

Successful merges:

Failed merges:

r? @ghost

richkadel and others added 30 commits May 21, 2020 18:10
update from origin 2020-06-10
Add needs-sanitizer-{address,leak,memory,thread} directive indicating
that test requires target with support for specific sanitizer.

This is an addition to the existing needs-sanitizer-support directive
indicating that test requires a sanitizer runtime library.
This should run much faster.

There are also some drive-by cleanups here to try to simplify things.
Also, the paths for in-tree crates are now displayed as relative
in `x.py test -h -v`.
This can live inside FnCtxt rather than ConfirmContext, and would be
useful for other operations as well.
This commit intends to fix an accidental regression from rust-lang#70846. The
goal of rust-lang#70846 was to build compiler-builtins with a maximal number of
CGUs to ensure that each module in the source corresponds to an object
file. This high degree of control for compiler-builtins is desirable to
ensure that there's at most one exported symbol per CGU, ideally
enabling compiler-builtins to not conflict with the system libgcc as
often.

In rust-lang#70846, however, only part of the compiler understands that
compiler-builtins is built with many CGUs. The rest of the compiler
thinks it's building with `sess.codegen_units()`. Notably the
calculation of `sess.lto()` consults `sess.codegen_units()`, which when
there's only one CGU it disables ThinLTO. This means that
compiler-builtins is built without ThinLTO, which is quite harmful to
performance! This is the root of the cause from rust-lang#73135 where intrinsics
were found to not be inlining trivial functions.

The fix applied in this commit is to remove the special-casing of
compiler-builtins in the compiler. Instead the build system is now
responsible for special-casing compiler-builtins. It doesn't know
exactly how many CGUs will be needed but it passes a large number that
is assumed to be much greater than the number of source-level modules
needed. After reading the various locations in the compiler source, this
seemed like the best solution rather than adding more and more special
casing in the compiler for compiler-builtins.

Closes rust-lang#73135
PR rust-lang#49778 introduced fs::canonicalize() which fails for a nonexistent path.
This is a surprise for someone used to GNU Autotools' configure which can create any necessary intermediate directories in prefix.

This change makes it run fs::create_dir_all() before canonicalize().
update from origin 2020-06-15
This initial version only injects counters at the top of each function.
Rust Coverage will require injecting additional counters at each
conditional code branch.
As suggested in PR feedback:

rust-lang#73011 (comment)

This allows count_code_region() to be handled like a normal intrinsic so
the InstanceDef::InjectedCode variant is no longer needed.
Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>
This commit modifies bootstrap so that `config.toml` is read first from
`RUST_BOOTSTRAP_CONFIG`, then `--config` and finally `config.toml` in the
current directory.

This is a subjective change, intended to improve the ergnomics when
using "development shells" for rustc development (for example, using tools
such as Nix) which set environment variables to ensure a reproducible
environment (these development shells can then be version controlled). By
optionally reading `config.toml` from an environment variable, a `config.toml`
can be defined in the development shell and a path to it exposed in the
`RUST_BOOTSTRAP_CONFIG` environment variable - avoiding the need to manually
symlink the contents of this file to `config.toml` in the working
directory.

Signed-off-by: David Wood <david@davidtw.co>
richkadel and others added 11 commits June 17, 2020 14:50
Fix up autoderef when reborrowing

Currently `(f)()` and `f.call_mut()` behaves differently if expression `f` contains autoderef in it. This causes a weird error in rust-lang#72225.

When `f` is type checked, `Deref` is used (this is expected as we can't yet determine if we should use `Fn` or `FnMut`). When subsequently we determine the actual trait to be used, when using the `f.call_mut()` syntax the `Deref` is patched to `DerefMut`, while for the `(f)()` syntax case it is not.

This PR replicates the fixup for the first case.

Fixes rust-lang#72225
Fixes rust-lang#68590
…sper

linker: MSVC supports linking static libraries as a whole archive
… r=tmandry

first stage of implementing LLVM code coverage

This PR replaces rust-lang#70680 (WIP toward LLVM Code Coverage for Rust) since I am re-implementing the Rust LLVM code coverage feature in a different part of the compiler (in MIR pass(es) vs AST).

This PR updates rustc with `-Zinstrument-coverage` option that injects the llvm intrinsic `instrprof.increment()` for code generation.

This initial version only injects counters at the top of each function, and does not yet implement the required coverage map.

Upcoming PRs will add the coverage map, and add more counters and/or counter expressions for each conditional code branch.

Rust compiler MCP rust-lang/compiler-team#278
Relevant issue: rust-lang#34701 - Implement support for LLVMs code coverage instrumentation

***[I put together some development notes here, under a separate branch.](https://github.com/richkadel/rust/blob/cfa0b21d34ee64e4ebee226101bd2ef0c6757865/src/test/codegen/coverage-experiments/README-THIS-IS-TEMPORARY.md)***
…akis

compiletest: Add directives to detect sanitizer support

Add needs-sanitizer-{address,leak,memory,thread} directive indicating
that test requires target with support for specific sanitizer.

This is an addition to the existing needs-sanitizer-support directive
indicating that test requires a sanitizer runtime library.

The existing needs-sanitizer-support directive could be incorporated into the
new ones, but I decided to retain it, since it enables running sanitizer
codegen tests even when building of sanitizer runtime libraries is disabled.
memory access sanity checks: abort instead of panic

Suggested by @Mark-Simulacrum, this should help reduce the performance impact of these checks.
…ins, r=Mark-Simulacrum

Change how compiler-builtins gets many CGUs

This commit intends to fix an accidental regression from rust-lang#70846. The
goal of rust-lang#70846 was to build compiler-builtins with a maximal number of
CGUs to ensure that each module in the source corresponds to an object
file. This high degree of control for compiler-builtins is desirable to
ensure that there's at most one exported symbol per CGU, ideally
enabling compiler-builtins to not conflict with the system libgcc as
often.

In rust-lang#70846, however, only part of the compiler understands that
compiler-builtins is built with many CGUs. The rest of the compiler
thinks it's building with `sess.codegen_units()`. Notably the
calculation of `sess.lto()` consults `sess.codegen_units()`, which when
there's only one CGU it disables ThinLTO. This means that
compiler-builtins is built without ThinLTO, which is quite harmful to
performance! This is the root of the cause from rust-lang#73135 where intrinsics
were found to not be inlining trivial functions.

The fix applied in this commit is to remove the special-casing of
compiler-builtins in the compiler. Instead the build system is now
responsible for special-casing compiler-builtins. It doesn't know
exactly how many CGUs will be needed but it passes a large number that
is assumed to be much greater than the number of source-level modules
needed. After reading the various locations in the compiler source, this
seemed like the best solution rather than adding more and more special
casing in the compiler for compiler-builtins.

Closes rust-lang#73135
… r=Mark-Simulacrum

bootstrap: read config from $RUST_BOOTSTRAP_CONFIG

This PR modifies bootstrap so that `config.toml` is read first from `RUST_BOOTSTRAP_CONFIG`, then `--config` and finally `config.toml` in the current directory.

This is a subjective change, intended to improve the ergnomics when using "development shells" for rustc development (for example, using tools such as Nix) which set environment variables to ensure a reproducible environment (these development shells can then be version controlled, e.g. [my rustc shell](https://github.com/davidtwco/veritas/blob/6b74a5c170b6efb2c7b094352932f9158f97eec0/nix/shells/rustc.nix)). By optionally reading `config.toml` from an environment variable, a `config.toml` can be defined in the development shell and a path to it exposed in the `RUST_BOOTSTRAP_CONFIG` environment variable - avoiding the need to manually symlink the contents of this file to `config.toml` in the working directory.
…ent-prefix, r=Mark-Simulacrum

bootstrap/install.rs: support a nonexistent `prefix` in `x.py install`

PR rust-lang#49778 introduced fs::canonicalize() which fails for a nonexistent path.
This is a surprise for someone used to GNU Autotools' configure which can create any necessary intermediate directories in prefix.

This change makes it run fs::create_dir_all() before canonicalize().
…mulacrum

Speed up bootstrap a little.

The bootstrap script was calling `cargo metadata` 3 times (or 6 with `-v`). This is a very expensive operation, and this attempts to avoid the extra calls. On my system, a simple command like `./x.py test -h -v` goes from about 3 seconds to 0.4.

An overview of the changes:

- Call `cargo metadata` only once with `--no-deps`. Optional dependencies are filtered in `in_tree_crates` (handling `profiler_builtins` and `rustc_codegen_llvm` which are driven by the config).
- Remove a duplicate call to `metadata::build` when using `-v`. I'm not sure why it was there, it looks like a mistake or vestigial from previous behavior.
- Remove check for `_shim`, I believe all the `_shim` crates are now gone.
- Remove check for `rustc_` and `*san` for `test::Crate::should_run`, these are no longer dependencies in the `test` tree.
- Use relative paths in `./x.py test -h -v` output.
- Some code cleanup (remove unnecessary `find_compiler_crates`, etc.).
- Show suite paths (`src/test/ui/...`) in `./x.py test -h -v` output.
- Some doc comments.
@RalfJung
Copy link
Member Author

@rustbot modify labels: +rollup
@bors r+ rollup=never p=10

@bors
Copy link
Contributor

bors commented Jun 19, 2020

📌 Commit 61c8925 has been approved by RalfJung

@rustbot rustbot added the rollup A PR which is a rollup label Jun 19, 2020
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 19, 2020
@bors
Copy link
Contributor

bors commented Jun 19, 2020

⌛ Testing commit 61c8925 with merge 72417d8...

@bors
Copy link
Contributor

bors commented Jun 19, 2020

☀️ Test successful - checks-azure
Approved by: RalfJung
Pushing 72417d8 to master...

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.