Don't try to remove assignments in SimplifyComparisonIntegral

[hanna-kruppe]

The pass tried to opportunistically clean up an unnecessary assign statement if there's no other use of the comparison folded into a switchInt. The reasoning was that switchInt(move _N) prevents uses of _N in other blocks. The immediate problem is that the pass didn't check for other uses of the comparison result in the same block. This PR drops the cleanup logic entirely to fix #158206.

One could try to salvage the cleanup by doing another scan through the basic block and looking for other uses of the temporary. However, the other half of the reasoning (move prevents later uses) is also dubious. As the documentation of Operand::Move says:

This may additionally overwrite the place with uninit bytes, depending on how we decide in UCG#188. You should not emit MIR that may attempt a subsequent second load of this place without first re-initializing it.

This does not justify assuming "moves makes place uninitialized" semantics for the purpose of optimizations. At least for now, it doesn't seem possible to do this cleanup soundly without a whole-function analysis that looks at all uses, i.e., a general dead code elimination pass.

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[rustbot]

r? @dingxiangfei2009

rustbot has assigned @dingxiangfei2009.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 19 candidates

[cjgillot]

Dead store elimination runs a second time after this pass, so leaving assignments around is not an issue.

@bors r+

[rust-bors]

📌 Commit a642d9b has been approved by cjgillot

It is now in the queue for this repository.

[ChrisDenton]

I've nominated this for beta and stable backport since I think #158206 has high severity.

[rustbot]

beta backport approved as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle. Backport labels are handled by them.

[rustbot]

stable backport approved as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle. Backport labels are handled by them.

[panstromek]

for #158228

@rust-timer build 6fd92a3

[rust-timer]

Finished benchmarking commit (6fd92a3): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 3.7%)

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

	mean	range	count
Regressions ❌ (primary)	3.7%	[2.4%, 4.3%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.7%	[2.4%, 4.3%]	3

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 480.522s -> 498.187s (3.68%)
Artifact size: 391.37 MiB -> 391.24 MiB (-0.03%)