coverage: Simplify parts of coverage graph creation
This is a combination of three semi-related simplifications to how coverage graphs are created, grouped into one PR to avoid conflicts.
There are no observable changes to the output of any of the coverage tests.
We only need to take action when the next block cannot be added to the current
chain, but the logic is much simpler if we express it in terms of when the
block _can_ be added.
continue `TypingMode` refactor
There are still quite a few places which (indirectly) rely on the `Reveal` of a `ParamEnv`, but we're slowly getting there
r? `@compiler-errors`
Mark `simplify_aggregate_to_copy` mir-opt as unsound
Mark the `simplify_aggregate_to_copy` mir-opt added in #128299 as unsound as it seems to miscompile the MCVE reported in https://github.com/rust-lang/rust/issues/132353. The mir-opt can be re-enabled once this case is fixed.
```rs
fn pop_min(mut score2head: Vec<Option<usize>>) -> Option<usize> {
loop {
if let Some(col) = score2head[0] {
score2head[0] = None;
return Some(col);
}
}
}
fn main() {
let min = pop_min(vec![Some(1)]);
println!("min: {:?}", min);
// panic happens here on beta in release mode
// but not in debug mode
min.unwrap();
}
```
This MCVE is included as a `run-pass` ui regression test in the first commit. I built the ui test with a nightly manually, and can reproduce the behavioral difference with `-C opt-level=0` and `-C opt-level=1`. Locally, this ui test will fail unless it was run on a compiler built with the second commit marking the mir-opt as unsound thus disabling it by default.
This PR **partially reverts** commit e7386b3, reversing changes made to 02b1be1. The mir-opt implementation is just marked as unsound but **not** reverted to make reland reviews easier. Test changes are **reverted if they were not pure additions**. Tests added by the original PR received `-Z unsound-mir-opts` compile-flags.
cc `@DianQK` `@cjgillot` (PR author and reviewer of #128299)
They represent a lot of abstraction and indirection, but they're only
used for `ConstAnalysis`, and apparently won't be used for any other
analyses in the future. This commit inlines and removes them, which
makes `ConstAnalysis` easier to read and understand.
Rename `rustc_abi::Abi` to `BackendRepr`
Remove the confabulation of `rustc_abi::Abi` with what "ABI" actually means by renaming it to `BackendRepr`, and rename `Abi::Aggregate` to `BackendRepr::Memory`. The type never actually represented how things are passed, as that has to have `PassMode` considered, at minimum, but rather it just is how we represented some things to the backend. This conflation arose because LLVM, the primary backend at the time, would lower certain IR forms using certain ABIs. Even that only somewhat was true, as it broke down when one ventured significantly afield of what is described by the System V AMD64 ABI either by using different architectures, ABI-modifying IR annotations, the same architecture **with different ISA extensions enabled**, or other... unexpected delights.
Unfortunately both names are still somewhat of a misnomer right now, as people have written code for years based on this misunderstanding. Still, their original names are even moreso, and for better or worse, this backend code hasn't received as much maintenance as the rest of the compiler, lately. Actually arriving at a correct end-state will simply require us to disentangle a lot of code in order to fix, much of it pointlessly repeated in several places. Thus this is not an "actual fix", just a way to deflect further misunderstandings.
This is a standard pattern:
```
MyAnalysis.into_engine(tcx, body).iterate_to_fixpoint()
```
`into_engine` and `iterate_to_fixpoint` are always called in pairs, but
sometimes with a builder-style `pass_name` call between them. But a
builder-style interface is overkill here. This has been bugging me a for
a while.
This commit:
- Merges `Engine::new` and `Engine::iterate_to_fixpoint`. This removes
the need for `Engine` to have fields, leaving it as a trivial type
that the next commit will remove.
- Renames `Analysis::into_engine` as `Analysis::iterate_to_fixpoint`,
gives it an extra argument for the optional pass name, and makes it
call `Engine::iterate_to_fixpoint` instead of `Engine::new`.
This turns the pattern from above into this:
```
MyAnalysis.iterate_to_fixpoint(tcx, body, None)
```
which is shorter at every call site, and there's less plumbing required
to support it.
The initial naming of "Abi" was an awful mistake, conveying wrong ideas
about how psABIs worked and even more about what the enum meant.
It was only meant to represent the way the value would be described to
a codegen backend as it was lowered to that intermediate representation.
It was never meant to mean anything about the actual psABI handling!
The conflation is because LLVM typically will associate a certain form
with a certain ABI, but even that does not hold when the special cases
that actually exist arise, plus the IR annotations that modify the ABI.
Reframe `rustc_abi::Abi` as the `BackendRepr` of the type, and rename
`BackendRepr::Aggregate` as `BackendRepr::Memory`. Unfortunately, due to
the persistent misunderstandings, this too is now incorrect:
- Scattered ABI-relevant code is entangled with BackendRepr
- We do not always pre-compute a correct BackendRepr that reflects how
we "actually" want this value to be handled, so we leave the backend
interface to also inject various special-cases here
- In some cases `BackendRepr::Memory` is a "real" aggregate, but in
others it is in fact using memory, and in some cases it is a scalar!
Our rustc-to-backend lowering code handles this sort of thing right now.
That will eventually be addressed by lifting duplicated lowering code
to either rustc_codegen_ssa or rustc_target as appropriate.
coverage: Don't rely on the custom traversal to find enclosing loops
This opens up the possibility of modifying or removing the custom graph traversal used in coverage counter creation, without losing access to the heuristics that care about a node's enclosing loops.
Actually changing the traversal is left for future work, because this PR on its own doesn't change the emitted coverage mappings at all.
Effects cleanup
- removed extra bits from predicates queries that are no longer needed in the new system
- removed the need for `non_erasable_generics` to take in tcx and DefId, removed unused arguments in callers
r? compiler-errors
- removed extra bits from predicates queries that are no longer needed in the new system
- removed the need for `non_erasable_generics` to take in tcx and DefId, removed unused arguments in callers
Then we can rename the _raw functions to drop their suffix, and instead
explicitly use is_stable_const_fn for the few cases where that is really what
you want.
Move `cmp_in_dominator_order` out of graph dominator computation
Dominator-order information is only needed for coverage graphs, and is easy enough to collect by just traversing the graph again.
This avoids wasted work when computing graph dominators for any other purpose.
Dominator-order information is only needed for coverage graphs, and is easy
enough to collect by just traversing the graph again.
This avoids wasted work when computing graph dominators for any other purpose.
coverage: Make counter creation handle node/edge counters more uniformly
Similar to #130380, this is another round of small improvements informed by my ongoing attempts to overhaul coverage counter creation.
One of the big benefits is getting rid of the awkward special-case that would sometimes attach an edge counter to a node instead. That was needed by the code that chooses which out-edge should be given a counter expression, but we can avoid that by making the corresponding check a little smarter.
I've also renamed several things to be simpler and more consistent, which should help with future changes.
Continue to get rid of `ty::Const::{try_}eval*`
This PR mostly does:
* Removes all of the `try_eval_*` and `eval_*` helpers from `ty::Const`, and replace their usages with `try_to_*`.
* Remove `ty::Const::eval`.
* Rename `ty::Const::normalize` to `ty::Const::normalize_internal`. This function is still used in the normalization code itself.
* Fix some weirdness around the `TransmuteFrom` goal.
I'm happy to split it out further; for example, I could probably land the first part which removes the helpers, or the changes to codegen which are more obvious than the changes to tools.
r? BoxyUwU
Part of https://github.com/rust-lang/rust/issues/130704
Dont ICE when computing coverage of synthetic async closure body
I'm not totally certain if this is *right*, but at least it doesn't ICE.
The issue is that we end up generating two MIR bodies for each async closure, since the `FnOnce` and `Fn`/`FnMut` implementations have different borrowing behavior of their captured variables. They should ideally both contribute to the coverage, since those MIR bodies are (*to the user*) the same code and should have no behavioral differences.
This PR at least suppresses the ICEs, and then I guess worst case we can fix this the right way later.
r? Zalathar or re-roll
Fixes#131190
Make destructors on `extern "C"` frames to be executed
This would make the example in #123231 print "Noisy Drop". I didn't mark this as fixing the issue because the behaviour is yet to be spec'ed.
Tracking:
- https://github.com/rust-lang/rust/issues/74990
Don't check unsize goal in MIR validation when opaques remain
Similarly to `mir_assign_valid_types`, let's just skip when there are opaques. Fixes#130921.
coverage: Multiple small tweaks to counter creation
I've been experimenting with some larger changes to how coverage counters are assigned to parts of the control-flow graph, and while none of that is ready yet, along the way I've repeatedly found myself wanting these smaller tweaks as a base.
There are no changes to compiler output.
Don't use Immediate::offset to transmute pointers to integers
This applies the relatively new `assert_matches_abi` check in the `offset` operation on immediates, which makes sure that if offsets are used to alter the layout (which is possible because the field layout is arbitrarily picked by the caller), this is not done in a way that breaks the invariant of the `Immediate` type.
This leads to ICEs in a GVN mir-opt test, so the second commit fixes GVN.
Fixes https://github.com/rust-lang/rust/issues/131064.
This makes it possible for other parts of counter-assignment to check whether a
node is guaranteed to end up with some kind of counter.
Switching from `impl Fn` to a concrete `&BitSet` just avoids the hassle of
trying to store a closure in a struct field, and currently there's no
foreseeable need for this information to not be a bitset.
