Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad size_hint() for Range<i64> on powerpc64le #72023

Closed
cuviper opened this issue May 8, 2020 · 5 comments
Closed

Bad size_hint() for Range<i64> on powerpc64le #72023

cuviper opened this issue May 8, 2020 · 5 comments
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-PowerPC Target: PowerPC processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cuviper
Copy link
Member

cuviper commented May 8, 2020

I tried this code:

fn main() {
    let range = -2..i64::MAX;
    dbg!(&range);
    // With this assertion, it works as expected!
    // assert!(range.start < range.end);
    dbg!(range.size_hint());
}

I expected to see this, which does work in debug mode:

[src/main.rs:3] &range = -2..9223372036854775807
[src/main.rs:6] range.size_hint() = (
    9223372036854775809,
    Some(
        9223372036854775809,
    ),
)

But in release mode, this happened:

[src/main.rs:3] &range = -2..9223372036854775807
[src/main.rs:6] range.size_hint() = (
    0,
    Some(
        0,
    ),
)

However, it does work in release mode if that start < len assertion is uncommented.

Meta

$ rustc --version --verbose
rustc 1.45.0-nightly (a08c47310 2020-05-07)
binary: rustc
commit-hash: a08c47310c7d49cbdc5d7afb38408ba519967ecd
commit-date: 2020-05-07
host: powerpc64le-unknown-linux-gnu
release: 1.45.0-nightly
LLVM version: 9.0

This is not new -- it comes from a failure noted in rayon-rs/rayon#585 in July 2018, which unfortunately I forgot to followup on until now. It's fine on x86_64, so it seems like an arch-specific codegen issue, most likely down to LLVM.

@cuviper cuviper added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-codegen Area: Code generation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. O-PowerPC Target: PowerPC processors C-bug Category: This is a bug. labels May 8, 2020
@cuviper
Copy link
Member Author

cuviper commented May 8, 2020

We have Rust built with LLVM 10 in Fedora 32, but that doesn't fix it:

$ rustc -Vv
rustc 1.43.0
binary: rustc
commit-hash: unknown
commit-date: unknown
host: powerpc64le-unknown-linux-gnu
release: 1.43.0
LLVM version: 10.0
$ cargo run 
   Compiling range_len v0.1.0 (/range_len)
    Finished dev [unoptimized + debuginfo] target(s) in 0.36s
     Running `target/debug/range_len`
[src/main.rs:3] &range = -2..9223372036854775807
[src/main.rs:6] range.size_hint() = (
    9223372036854775809,
    Some(
        9223372036854775809,
    ),
)
$ cargo run --release
   Compiling range_len v0.1.0 (/range_len)
    Finished release [optimized] target(s) in 0.30s
     Running `target/release/range_len`
[src/main.rs:3] &range = -2..9223372036854775807
[src/main.rs:6] range.size_hint() = (
    0,
    Some(
        0,
    ),
)

@programmerjake
Copy link
Member

I didn't actually run it on a Power processor to test, but AFAICT the bug reduces to this:

pub fn f(a: i64, b: i64) -> u64 {
    if a < b {
        0
    } else {
        a.wrapping_sub(b) as u64
    }
}

LLVM miscompiles it assuming that a.wrapping_sub(b) won't have signed overflow.

https://rust.godbolt.org/z/49fncx

Output assembly:

example::f:
        sub.    3, 3, 4
        isellt  3, 0, 3
        blr
        .long   0
        .quad   0

the sub. sets the condition register CR0 based on sign of wrapped result, not on if r3 is less than, equal to, or greater than r4.

Will open a LLVM bug report shortly.

@programmerjake
Copy link
Member

Submitted: https://bugs.llvm.org/show_bug.cgi?id=47830

@ecnelises
Copy link
Contributor

It should be fixed by llvm/llvm-project@f6515b0 , @cuviper can you verify it? Thanks.

@cuviper
Copy link
Member Author

cuviper commented Jan 6, 2022

Looks like that fix released in LLVM 12. I just tried the reproducer again on Fedora 33, 34, and 35, with LLVM 11, 12, and 13 respectively -- it failed with 11 and passed with 12 and 13. Our minimum is 12 since #90175, so I think we're good!

@cuviper cuviper closed this as completed Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-PowerPC Target: PowerPC processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants