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 #73669

Merged
merged 37 commits into from
Jun 24, 2020
Merged

Rollup of 11 pull requests #73669

merged 37 commits into from
Jun 24, 2020

Conversation

Manishearth
Copy link
Member

Successful merges:

Failed merges:

r? @ghost

GuillaumeGomez and others added 30 commits May 30, 2020 15:36
I think it would be nice to mention this, so you don't have to dig through the src to look at the definition of new().
Replaced dummy values for hash and num_counters with computed values,
and refactored InstrumentCoverage pass to simplify injecting more
counters per function in upcoming versions.

Improved usage documentation and error messaging.
This is to prevent the miscompilation in rust-lang#73137 from reappearing.
Only runs with `-Zvalidate-mir`.
It turns out that this has not been working for who knows how long.
Previously:

```
pub fn h() { 1 + 2; }
```

After this change:

```
pub fn h() { loop {} }
```

This only affected the pass when run with the command line
pretty-printing option, so rustdoc was still replacing bodies with
`loop {}`.
This improves the output for issue rust-lang#72577, but there's still more work
to be done.

Currently, an overflow error during monomorphization results in an error
that points at the function we were unable to monomorphize. However, we
don't point at the call that caused the monomorphization to happen. In
the overflow occurs in a large recursive function, it may be difficult
to determine where the issue is.

This commit tracks and `Span` information during collection of
`MonoItem`s, which is used when emitting an overflow error. `MonoItem`
itself is unchanged, so this only affects
`src/librustc_mir/monomorphize/collector.rs`
This commit adds a query that allows the CoverageData to be pulled from
a call on tcx, avoiding the need to change the
`codegen_intrinsic_call()` signature (no need to pass in the FunctionCx
or any additional arguments.

The commit does not change where/when the CoverageData is computed. It's
still done in the `pass`, and saved in the MIR `Body`.

See discussion (in progress) here:
rust-lang#73488 (comment)
The mod uses both MIR bodies and HIR bodies, so I'm trying to maintain
consistency with these names.
Also added FIXME comments to note the possible need to accommodate
counter increment calls in source-based functions that differ from the
function context of the caller instance (e.g., inline functions).
Thus we avoid propagation of a local the moment we encounter references to it.
This commit modernizes how rustc checks for whether the `atomics`
feature is enabled for the wasm target. The `sess.target_features` set
is consulted instead of fiddling around with dealing with various
aspects of LLVM and that syntax.
Mention that BTreeMap::new() doesn't allocate

I think it would be nice to mention this, so you don't have to dig through the src to look at the definition of new().
…ir, r=tmandry

Check for assignments between non-conflicting generator saved locals

This is to prevent future changes to the generator transform from reintroducing the problem that caused rust-lang#73137. Namely, a store between two generator saved locals whose storage does not conflict.

My ultimate goal is to introduce a modified version of rust-lang#71956 that handles this case properly.

r? @tmandry
…tmandry

code coverage foundation for hash and num_counters

This PR is the next iteration after PR rust-lang#73011 (which is still waiting on bors to merge).

@wesleywiser - PTAL
r? @tmandry

(FYI, I'm also working on injecting the coverage maps, in another branch, while waiting for these to merge.)

Thanks!
…morse

Fix -Z unpretty=everybody_loops

It turns out that this has not been working for who knows how long.
Previously:

```
pub fn h() { 1 + 2; }
```

After this change:

```
pub fn h() { loop { } }
```

This only affected the pass when run with the command line
pretty-printing option, so rustdoc was still replacing bodies with
`loop {}`.
… r=petrochenkov

Move remaining `NodeId` APIs from `Definitions` to `Resolver`

Implements rust-lang#73291 (comment)

TL;DR: it moves all fields that are only needed during name resolution passes into the `Resolver` and keep the rest in `Definitions`. This effectively enforces that all references to `NodeId`s are gone once HIR lowering is completed.

After this, the only remaining work for rust-lang#50928 should be to adjust the dev guide.

r? @petrochenkov
…err, r=ecstatic-morse

Point at the call span when overflow occurs during monomorphization

This improves the output for issue rust-lang#72577, but there's still more work
to be done.

Currently, an overflow error during monomorphization results in an error
that points at the function we were unable to monomorphize. However, we
don't point at the call that caused the monomorphization to happen. In
the overflow occurs in a large recursive function, it may be difficult
to determine where the issue is.

This commit tracks and `Span` information during collection of
`MonoItem`s, which is used when emitting an overflow error. `MonoItem`
itself is unchanged, so this only affects
`src/librustc_mir/monomorphize/collector.rs`
…sleywiser

The const propagator cannot trace references.

Thus we avoid propagation of a local the moment we encounter references to it.

fixes rust-lang#73609

cc @RalfJung

r? @wesleywiser
…ature, r=davidtwco

rustc: Modernize wasm checks for atomics

This commit modernizes how rustc checks for whether the `atomics`
feature is enabled for the wasm target. The `sess.target_features` set
is consulted instead of fiddling around with dealing with various
aspects of LLVM and that syntax.
@Manishearth
Copy link
Member Author

@bors r+ p=4 rollup=never

@bors
Copy link
Contributor

bors commented Jun 23, 2020

📌 Commit 6ed6a84 has been approved by Manishearth

@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 23, 2020
@bors
Copy link
Contributor

bors commented Jun 23, 2020

⌛ Testing commit 6ed6a84 with merge 0c04344...

@bors
Copy link
Contributor

bors commented Jun 24, 2020

☀️ Test successful - checks-azure
Approved by: Manishearth
Pushing 0c04344 to master...

@nnethercote
Copy link
Contributor

This was a perf loss of up to 2.1% on a few benchmarks, mostly on incr-unchanged runs.

I have no idea which PR(s) might be responsible.

@Manishearth Manishearth deleted the rollup-0n4u7vq branch June 30, 2020 00:29
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
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. 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.