AddNicheCases MirPass

[mikebenfield]

This pass optimizes switches on the discriminant of a niche-optimized enum.

[rust-highfive]

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

[rust-highfive]

r? @wesleywiser

(rust-highfive has picked a reviewer for you, use r? to override)

[mikebenfield]

This was part of PR #94075, but it was requested that I split this and the actual niche-filling optimization into two parts.

[oli-obk]

Considering the crater results from the original PR, I think we should run this PR against all the tests that failed

[RalfJung]

Wait, this is an optimization, why would it have any impact on crate test failures?

[JakobDegen]

This seems like it removes the assignment to d without proving that it's unused later on, which is unsound. Not removing the assignment could fix this.

However, I have more general concerns about this going into MIR opts. MIR is generally supposed to be layout independent, and it being pre-monomorphization means that this opt sometimes won't fire for not great reasons. Especially the prospect of changing one of the key types (Rvalue) specifically to enable this kind of layout dependent opt is something I do not think is a good idea.

As best I can tell, there's no reason to believe that LLVM won't be able to figure this out on its own - we may indeed emit worse code, but at least this will only affect compile times and not run time of the compiled artifact. If we to avoid emitting the worse code that of course seems like a laudable goal, but imo it belongs in codegen.

[mikebenfield]

LLVM certainly does not figure this out currently. And I'm not sure it can in general. I'll have to revisit this when I can sit down and think about it, but when LLVM IR is emitted, information about what are valid values for the niche is lost, so LLVM doesn't know there's only a handful of extra branches it would need to create.

That said, I take your point about RValue and the appropriateness of doing this in MIR. I'll look into whether I can do something similar in codegen instead.

[JakobDegen]

Yeah, I had misunderstood a message in the other PR as implying this was just for compile times. Imo this makes it even more important that this fires consistently though

[bors]

☔ The latest upstream changes (presumably #95966) made this pull request unmergeable. Please resolve the merge conflicts.

[mikebenfield]

Closing this as it seems it's no longer necessary for performance.