Miscompilation due to MatchBranchSimplification MIR pass mixing up discriminants about rust HOT 13 CLOSED

yeah, could you put up a quick PR to disable it, gating it behind unsound MIR opts?

I will do it today. :)

from rust.

I think disabling the entire pass should have a very small impact, it's not like this is the LLVM instcombine pass :D. So that makes more sense, it's usually how we do it for less important MIR opt passes.

from rust.

#124156 landed so the soundness bug is fixed by disabling the affected optimization. Thanks @DianQK for the quick fix. :)

from rust.

Cc @rust-lang/wg-mir-opt

from rust.

does it make sense to quickly throw up a PR disabling it to leave more time for fixing it properly and thoroughly?

from rust.

does it make sense to quickly throw up a PR disabling it to leave more time for fixing it properly and thoroughly?

I like this: #124122 (comment).
I think that disabling some transforms where miscompilation might occur is feasible in #124122.

from rust.

yeah, could you put up a quick PR to disable it, gating it behind unsound MIR opts?

from rust.

For me, but it's more appropriate to disable unsigned and signed conversions in #124122. But might this require a longer review? I'm not sure, but at least the conversions that are retained should be clear.
@oli-obk Do you have any suggestions?

from rust.

For me, but it's more appropriate to disable unsigned and signed conversions in #124122.

This is just the usual operation on LLVM: disabling certain conversions, rather than disabling the entire Pass. Of course, compared to LLVM's passes, the passes in MIR-opt are much smaller.

from rust.

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-critical

from rust.

yeah, could you put up a quick PR to disable it, gating it behind unsound MIR opts?

I've rethought this scenario, and it's appropriate to add unsound opts. Because this pass is mostly all in #120614. :)

from rust.

I caused a P-critical issue for the first time. :p

from rust.

mir-opts work has a pretty high chance of doing that. Unfortunately we haven't yet figured out how to build correct-by-construction optimization passes outside of full-blown theorem provers...

from rust.