Enable DestinationPropagation by default
This PR proposes to perform destination propagation on MIR. Most of the pass was fully rewritten by `@JakobDegen` in rust-lang/rust#96451.
This pass is quite heavy, as it needs to perform and save the results of a full liveness dataflow analysis. This accounts for ~50% of the pass' runtime.
Perf sees a few decent savings in later llvm passes, but also sizeable régressions when there are no savings to balance this pass' runtime.
Remove Rvalue::Len again.
Now that we have `RawPtrKind::FakeForPtrMetadata`, we can reimplement `Rvalue::Len` using `PtrMetadata(&raw const (fake) place)`.
r? ``@scottmcm``
Reimplement DestinationPropagation according to live ranges.
This PR reimplements DestinationPropagation as a problem of merging live-ranges of locals. We merge locals that have disjoint live-ranges. This allows merging several locals in the same round by updating live range information.
Live ranges are mainly computed using the `MaybeLiveLocals` analysis. The subtlety is that we split each statement and terminator in 2 positions. The first position is the regular statement. The second position is a shadow, which is always more live. It encodes partial writes and dead writes as a local being live for half a statement. This half statement ensures that writes conflict with another local's writes and regular liveness.
r? `@Amanieu`
Convert moves of references to copies in ReferencePropagation
This is a fix for https://github.com/rust-lang/rust/issues/141101.
The root cause of this miscompile is that the SsaLocals analysis that MIR transforms use is supposed to detect locals that are only written to once, in their single assignment. But that analysis is subtly wrong; it does not consider `Operand::Move` to be a write even though the meaning ascribed to `Operand::Move` (at least as a function parameter) by Miri is that the callee may have done arbitrary writes to the caller's Local that the Operand wraps (because `Move` is pass-by-pointer). So Miri conwiders `Operand::Move` to be a write but both the MIR visitor system considers it a read, and so does SsaLocals.
I have tried fixing this by changing the `PlaceContext` that is ascribed to an `Operand::Move` to a `MutatingUseContext` but that seems to have borrow checker implications, and changing SsaLocals seems to have wide-ranging regressions in MIR optimizations.
So instead of doing those, this PR adds a new kludge to ReferencePropagation, which follows the same line of thinking as the kludge in CopyProp that solves this same problem inside that pass: a5584a8fe1/compiler/rustc_mir_transform/src/copy_prop.rs (L65-L98)
This removes the #[no_sanitize] attribute, which was behind an unstable
feature named no_sanitize. Instead, we introduce the sanitize attribute
which is more powerful and allows to be extended in the future (instead
of just focusing on turning sanitizers off).
This also makes sanitize(kernel_address = ..) attribute work with
-Zsanitize=address
To do it the same as how clang disables address sanitizer, we now
disable ASAN on sanitize(kernel_address = "off") and KASAN on
sanitize(address = "off").
The same was added to clang in https://reviews.llvm.org/D44981.
Remove unneeded `drop_in_place` calls
Might as well pull this out from rust-lang/rust#144561 because this is still used in things like `Vec::truncate` where it'd be nice to allow it be removed if inlined enough to see that the type is `Copy`.
So long as perf says it's ok, at least 🤞
lower pattern bindings in the order they're written and base drop order on primary bindings' order
To fixrust-lang/rust#142163, this PR does two things:
- Makes match arms base their drop order on the first sub-branch instead of the last sub-branch. Together with the second change, this makes bindings' drop order correspond to the relative order of when each binding first appears (i.e. the order of the "primary" bindings).
- Lowers pattern bindings in the order they're written (still treating the right-hand side of a ``@`` as coming before the binding on the left). In each sub-branch of a match arm, this is the order that would be obtained if the or-alternatives chosen in that sub-branch were inlined into the arm's pattern. This both affects drop order (making bindings in or-patterns not be dropped first) and fixes the issue in [this test](2a023bf80a/tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.rs) from rust-lang/rust#121716.
My approach to the second point is admittedly a bit trickier than may be necessary. To avoid passing around a counter when building `FlatPat`s, I've instead added just enough information to recover the original structure of the pattern's bindings from a `MatchTreeSubBranch`'s path through the `Candidate` tree. Some alternatives:
- We could use a counter, then sort bindings by their ordinals when making `MatchTreeSubBranch`es.
- I'd like to experiment with always merging sub-candidates and removing `test_remaining_match_pairs_after_or`; that would require lowering bindings and guards in a different way. That makes it a bigger change too, though, so I figure it might be simplest to start here.
- For a very big change, we could track which or-alternatives succeed at runtime to base drop order on the binding order in the particular alternatives matched.
This is a breaking change. It will need a crater run, language team sign-off, and likely updates to the Reference.
This will conflict with rust-lang/rust#143376 and probably also rust-lang/rust#143028, so they shouldn't be merged at the same time.
r? `@matthewjasper` or `@Nadrieril`
emit `StorageLive` and schedule `StorageDead` for `let`-`else`'s bindings after matching
This PR removes special handling of `let`-`else`, so that `StorageLive`s are emitted and `StorageDead`s are scheduled only after pattern-matching has succeeded. This means `StorageDead`s will no longer appear for all of its bindings on the `else` branch (because they're not live yet) and its drops&`StorageDead`s will happen together like they do elsewhere, rather than having all drops first, then all `StorageDead`s.
This fixesrust-lang/rust#142056, and is therefore a breaking change. I believe it'll need a crater run and a T-lang nomination/fcp thereafter. Specifically, this makes drop-checking slightly more restrictive for `let`-`else` to match the behavior of other variable binding forms. An alternative approach could be to change the relative order of drops and `StorageDead`s for other binding forms to make drop-checking more permissive, but making that consistent would be a significantly more involved change.
r? mir
cc `````@dingxiangfei2009`````
`````@rustbot````` label +T-lang +needs-crater
Add some pre-codegen MIR tests for debug mode
No functional changes; just some tests.
I made these for rust-lang/rust#144483, but that's going in a different direction, so I wanted to propose we just add them to help see the impact of other related changes in the future.
r? mir
`AlignmentEnum` should just be `repr(usize)` now
These used to use specific sizes because they were compiled on all widths. But now that the types themselves are `#[cfg]`'d, we can save some conversions by having it always be `repr(usize)`.
Rollup of 3 pull requests
Successful merges:
- rust-lang/rust#144657 (fix: Only "close the window" when its the last annotated file)
- rust-lang/rust#144665 (Re-block SRoA on SIMD types)
- rust-lang/rust#144713 (`rustc_middle::ty` cleanups)
r? `@ghost`
`@rustbot` modify labels: rollup
coverage: Re-land "Enlarge empty spans during MIR instrumentation"
This allows us to assume that coverage spans will only be discarded during codegen in very unusual situations.
---
This seemingly-simple change has a rather messy history:
- rust-lang/rust#140847
- rust-lang/rust#141650
- rust-lang/rust#144298
- rust-lang/rust#144480
Since then, a number of related changes have landed that should make it reasonable to try again:
- rust-lang/rust#144530
- rust-lang/rust#144560
- rust-lang/rust#144616
In particular, we have multiple fixes/mitigations, and a confirmed regression test for the original bug that is not triggered by re-landing the changes in this PR.
Fix Box allocator drop elaboration
New version of rust-lang/rust#131146.
Clearing Box's drop flag after running its destructor can cause it to skip dropping its allocator, so just don't. Its cleared by the drop ladder code afterwards already.
Unlike the last PR this also handles other types with destructors properly, in the event that we can have open drops on them in the future (by partial initialization or DerefMove or something).
Finally, I also added tests for the interaction with async drop here but I discovered rust-lang/rust#143658, so one of the tests has a `knownbug` annotation. Not sure if it should be in this PR at all though.
Fixesrust-lang/rust#131082
r? wesleywiser - prev. reviewer
