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

Regression on nightly in AVX2 byte shift intrinsics #85446

Closed
marcgalois opened this issue May 18, 2021 · 5 comments · Fixed by #85493
Closed

Regression on nightly in AVX2 byte shift intrinsics #85446

marcgalois opened this issue May 18, 2021 · 5 comments · Fixed by #85493
Assignees
Labels
C-bug Category: This is a bug. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections O-x86_64 Target: x86-64 processors (like x86_64-*) P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@marcgalois
Copy link

Code

I tried this code:

use std::arch::x86_64::*;

fn main() {
    let input: [u8; 32] = [
        1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25,
        26, 27, 28, 29, 30, 31, 32,
    ];
    let output: [u8; 32] = unsafe {
        let input: __m256i = std::mem::transmute(input);
        let output = _mm256_bslli_epi128(input, 8);
        std::mem::transmute(output)
    };
    dbg!(output);
}

I expected to see this:

[src/main.rs:13] output = [ 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3, 4, 5, 6, 7, 8, 0, 0, 0, 0, 0, 0, 0, 0, 17, 18, 19, 20, 21, 22, 23, 24]

(which matches clang on the equivalent C++ code)

Instead, this happened:

[src/main.rs:13] output = [ 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3, 4, 5, 6, 7, 8, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3, 4, 5, 6, 7, 8]

Version it worked on

It most recently worked on: Stable rust 1.52.1

Version with regression

rustc --version --verbose:

rustc 1.54.0-nightly (fe72845f7 2021-05-16)
binary: rustc
commit-hash: fe72845f7bb6a77b9e671e6a4f32fe714962cec4
commit-date: 2021-05-16
host: x86_64-apple-darwin
release: 1.54.0-nightly
LLVM version: 12.0.1

I suspect that this issue was caused by rust-lang/stdarch#1067

@marcgalois marcgalois added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels May 18, 2021
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-untriaged Untriaged performance or correctness regression. labels May 18, 2021
@LeSeulArtichaut LeSeulArtichaut added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label May 18, 2021
@LeSeulArtichaut
Copy link
Contributor

Let's bisect this: @rustbot ping cleanup

@rustbot
Copy link
Collaborator

rustbot commented May 18, 2021

@rustbot rustbot added the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label May 18, 2021
@LeSeulArtichaut LeSeulArtichaut added the O-x86_64 Target: x86-64 processors (like x86_64-*) label May 18, 2021
@LeSeulArtichaut
Copy link
Contributor

LeSeulArtichaut commented May 18, 2021

(I ended up doing the bisection myself, sorry for the ping everyone!)

cargo-bisect-rustc finds a regression in #83278 (which does contain rust-lang/stdarch#1067), cc @Amanieu @Mark-Simulacrum

@Mark-Simulacrum Mark-Simulacrum added this to the 1.54.0 milestone May 18, 2021
@Amanieu
Copy link
Member

Amanieu commented May 18, 2021

cc @minybot

@jyn514 jyn514 added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label May 19, 2021
@apiraino
Copy link
Contributor

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-critical +T-compiler

@rustbot rustbot added P-critical Critical priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 19, 2021
@Amanieu Amanieu self-assigned this May 19, 2021
bors added a commit to rust-lang-ci/rust that referenced this issue May 20, 2021
Update stdarch to fix x86 byte shift intrinsics

Fixes rust-lang#85446
@bors bors closed this as completed in d4a0bcc May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections O-x86_64 Target: x86-64 processors (like x86_64-*) P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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.

7 participants