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

[MIPS] AtomicI8 and AtomicI16 doctests for fn fetch_min() and fn fetch_max() began failing in nightly-2024-04-08 #123772

Closed
martn3 opened this issue Apr 11, 2024 · 2 comments · Fixed by #124044
Labels
A-atomic Area: atomics, barriers, and sync primitives A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-MIPS Target: MIPS processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@martn3
Copy link

martn3 commented Apr 11, 2024

Hello,

nightly-2024-04-08 regressed on the following tests on MIPS:

failures:
    library/core/src/sync/atomic.rs - sync::atomic::AtomicI16::fetch_max (line 3072)
    library/core/src/sync/atomic.rs - sync::atomic::AtomicI16::fetch_min (line 3072)
    library/core/src/sync/atomic.rs - sync::atomic::AtomicI16::fetch_min (line 3084)
    library/core/src/sync/atomic.rs - sync::atomic::AtomicI8::fetch_max (line 3036)
    library/core/src/sync/atomic.rs - sync::atomic::AtomicI8::fetch_min (line 3036)
    library/core/src/sync/atomic.rs - sync::atomic::AtomicI8::fetch_min (line 3048)

test result: FAILED. 4537 passed; 6 failed; 37 ignored; 0 measured; 6 filtered out; finished in 315.73s

I have bisected this to #123555 in the Rust repo and to llvm/llvm-project@fbb27d1 in the LLVM repo.

How to reproduce

  1. (On Debian 12) install a MIPS toolchain:
sudo apt install \
    gcc-mipsel-linux-gnu \
    libc6-mipsel-cross \
    qemu-user
  1. Create a file main.rs with the following contents (which is a simplified snippet of the failing doctests mentioned above):
fn main() {
    let x = std::sync::atomic::AtomicI8::new(23);
    assert_eq!(x.fetch_max(42, std::sync::atomic::Ordering::SeqCst), 23);
}
  1. Build it like this:
./x build --target mipsel-unknown-linux-gnu
build/x86_64-unknown-linux-gnu/stage1/bin/rustc \
    --target mipsel-unknown-linux-gnu \
    -Clinker=mipsel-linux-gnu-gcc \
    main.rs
  1. Run like this:
qemu-mipsel -L /usr/mipsel-linux-gnu main

Expected

The program completes successfully.

Actual

The program fails like this:

thread 'main' panicked at src/main.rs:3:5:
assertion `left == right` failed
  left: 0
 right: 23
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Bisection

With src/llvm-project pointing to llvm/llvm-project@fbb27d1 the test also fails, but when pointing src/llvm-project to the parent commit (llvm/llvm-project@e74c167) the test passes.

So we can conclude that this regression was introduced by llvm/llvm-project@fbb27d1, which originates from llvm/llvm-project#77072.

Edit: See llvm/llvm-project#77072 for more discussion around this.

@rustbot label +O-MIPS +A-atomic +T-compiler +C-bug

@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-atomic Area: atomics, barriers, and sync primitives C-bug Category: This is a bug. O-MIPS Target: MIPS processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 11, 2024
@DianQK
Copy link
Member

DianQK commented Apr 11, 2024

@rustbot label +A-LLVM

@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Apr 11, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 11, 2024
@bors bors closed this as completed in 9f432d7 Apr 18, 2024
jdmitrovic-syrmia added a commit to jdmitrovic-syrmia/llvm-project that referenced this issue Apr 18, 2024
In order for the following `SLT` instruction to work properly,
we need to sign-extend appropriate subwords.

In addition, subwords must remain in the same position from
before sign-extension.

Resolves llvm#61881. Also, downstream bugs rust-lang/rust#100650 and
rust-lang/rust#123772 are fixed.
jdmitrovic-syrmia added a commit to jdmitrovic-syrmia/llvm-project that referenced this issue Apr 19, 2024
In order for the following `SLT` instruction to work properly,
we need to sign-extend appropriate subwords.

In addition, subwords must remain in the same position from
before sign-extension.

Resolves llvm#61881. Also, downstream bugs rust-lang/rust#100650 and
rust-lang/rust#123772 are fixed.
@martn3
Copy link
Author

martn3 commented Apr 19, 2024

I can confirm that the regression has been fixed 👍

The first nightly with the fix is nightly-2024-04-19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-atomic Area: atomics, barriers, and sync primitives A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-MIPS Target: MIPS processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants