Rust 1.56.0+ no longer recognizes boundary checks as avoiding division overflow panic

[mqudsi]

The following code (contrived for demonstration purposes) correctly checks for and avoids a division overflow condition (i64::MIN input coerced to i64::MIN.abs() output) but a regression in rustc 1.56.0 (still in latest nightly and latest 1.62.0 release) causes the optimizer to no longer skip the checked division and panic branch:

pub fn checked_div_i64(dividend: i64, divisor: i64) -> Option<i64> {
    if dividend > i64::min_value() && divisor != 0 {
        Some(dividend / divisor)
    } else {
        None
    }
}

Godbolt link comparing 1.55.0 to 1.62.0

@rustbot label +regression-from-stable-to-stable +A-codegen +A-llvm +I-heavy +I-slow +T-compiler

Edit:

It might be worth adding some regression tests for this because it's happened and then was fixed a few times before (e.g. introduced in 1.47.0 and fixed in 1.48.0).

[mqudsi]

This bisects to something between 2021-08-21 and 2021-08-22 in the range of a003591 to d3e2578, which includes #87570 (the upgrade to llvm 13), which is the most likely culprit.

[workingjubilee]

The only difference in our unoptimized LLVMIR is that we now emit noundef, which enables other optimizations. Typically we do not add regression tests to Rust's test suite based purely on LLVM optimizations, and this regression does seem to be due to that. Even if we wanted one here, there's so many valid code emissions and optimizations for division, I'm not seeing a simple path forward for a regression test here.

But we will be upgrading LLVM soon (work has commenced in #99464).

[mqudsi]

Thanks for clarifying - I wasn't sure what was in-scope and what would purely be considered an upstream issue without any workarounds on our end. I should have checked --emit=llvm-ir and included that info in my original bug report - I've made a mental note to do that in the future.

I look forward to seeing if the migration to LLVM 15 will address this. In the meantime, I suppose this is what core::hint::unreachable_unchecked() was made for!

[nikic]

Still fails to optimize on current LLVM: https://llvm.godbolt.org/z/n7x4e9TKP It does work if the first logical and is replaced with a bitwise and, so something is missing logical and handling.

[workingjubilee]

It's a regression, so I see some value in tracking it on our issue tracker, I just don't see much value in a specific test for it because for the optimized forms of the LLVMIR, I thiiiink the only thing we could really do is check that bb6 is not present: https://godbolt.org/z/M9brfTcj9

[nikic]

Even this simple case doesn't get optimized: https://alive2.llvm.org/ce/z/jvVFVJ

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-low

[nikic]

Fixed since 1.68, though I don't know specifically what fixed it.

[apiraino]

thanks! Tentatively closing the issue. Please @mqudsi report back if you still experience the issue

[mqudsi]

Thanks @nikic for the update and the codegen regression tests.