Update MaybeUninitializedPlaces and MaybeInitializedPlaces for otherwise

[ashivaram23]

This allows ElaborateDrops to remove drops when a match wildcard arm covers multiple no-Drop enum variants. It modifies dataflow analysis to update the MaybeUninitializedPlaces and MaybeInitializedPlaces data for a block reached through an otherwise edge.

This appears to fix #142705, but I don't know for sure if it's actually correct (are there cases where this would be wrong and break things?). I also haven't tested that it actually improves compile times, or machine code output outside of the examples in the issue.

[rustbot]

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[petrochenkov]

r? compiler

[workingjubilee]

hm.
@bors2 try @rust-timer queue

[rust-bors]

⌛ Trying commit 5837955 with merge e3d7e41…

To cancel the try build, run the command @bors2 try cancel.

[rust-bors]

☀️ Try build successful (CI)
Build commit: e3d7e41 (e3d7e41d7e8c513f55398f6e34eb2af7fb7df49f, parent: 8de4c7234dd9b97c9d76b58671343fdbbc9a433e)

[rust-timer]

Finished benchmarking commit (e3d7e41): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	1.1%	[0.6%, 1.5%]	2
Regressions ❌ (secondary)	0.6%	[0.4%, 0.8%]	4
Improvements ✅ (primary)	-0.5%	[-1.2%, -0.2%]	32
Improvements ✅ (secondary)	-0.7%	[-1.3%, -0.2%]	19
All ❌✅ (primary)	-0.4%	[-1.2%, 1.5%]	34

Max RSS (memory usage)

Results (primary -0.2%, secondary 2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.5%	[2.3%, 4.7%]	3
Regressions ❌ (secondary)	2.8%	[2.2%, 3.5%]	2
Improvements ✅ (primary)	-2.4%	[-3.6%, -0.5%]	5
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.2%	[-3.6%, 4.7%]	8

Cycles

Results (primary 0.9%, secondary -5.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.7%	[2.6%, 2.9%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.0%	[-1.2%, -0.8%]	2
Improvements ✅ (secondary)	-5.0%	[-5.0%, -5.0%]	1
All ❌✅ (primary)	0.9%	[-1.2%, 2.9%]	4

Binary size

Results (primary -0.1%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.2%]	7
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	7
Improvements ✅ (primary)	-0.1%	[-0.5%, -0.0%]	64
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.0%]	50
All ❌✅ (primary)	-0.1%	[-0.5%, 0.2%]	71

Bootstrap: 691.904s -> 692.572s (0.10%)
Artifact size: 372.02 MiB -> 371.98 MiB (-0.01%)

[ashivaram23]

Hmm that's unfortunate. It might help to avoid paying the cost of the extra clone and checks when it won't make a difference, like in the trivial otherwise targets when there's no wildcard. Maybe the otherwise could be handled in a separate pass after collecting variants to kill/add in a SmallVec, in case that helps somehow.

What's more concerning is runtime benchmarks being worse. There are only two, and I don't know how noisy they are, but 8% wall time increase for css-parse-fb is pretty bad. There are also some compile time benchmarks whose graphs show significant increases in wall time spent in LLVM. Does this just not play well with later optimizations?

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[ashivaram23]

The last commit makes it so that there's an option in the MaybeUninitializedPlaces and MaybeInitializedPlaces builders for whether or not to update the otherwise block's lattice element, and it's set to only do so for MaybeInitializedPlaces in RemoveUninitDrops and ElaborateDrops. That should be enough to clean up some drops while hopefully being faster and not messing with other passes in ways that cause runtime slowdowns.

[workingjubilee]

the runtime benchmarks are unfortunately very noisy

[fee1-dead]

I wonder if it will be better, perf-wise to make this not take an option but just use a no-op for callers that do not want to handle it.

[fee1-dead]

this part of MIR is a little above my level of confidence for review, so

r? compiler