Skip to content

rustfmt breaks stdsimd #2616

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

Closed
gnzlbg opened this issue Apr 11, 2018 · 3 comments
Closed

rustfmt breaks stdsimd #2616

gnzlbg opened this issue Apr 11, 2018 · 3 comments
Labels
a-macros bug Panic, non-idempotency, invalid code, etc.

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 11, 2018

git clone [email protected]:rust-lang-nursery/stdsimd.git
cd stdsimd/crates/coresimd
cargo test # wait...
cargo fmt --all
cargo test # BOOM

250 errors of the form:

error: no rules expected the token `,`
   --> crates/coresimd/src/../../../coresimd/x86/avx2.rs:478:32
    |
478 |                     $f2, 14, 15,
    |                                ^
...
536 |         _ => blend1!(16, 17, 24, 25),
    |              ----------------------- in this macro invocation

Small snippet before:

 0b00 => blend4!(
                    $a,
                    $b,
                    $c,
                    $d,
                    $e,
                    $f,
                    6,
                    7,
                    $a2,
                    $b2,
                    $c2,
                    $d2,
                    $e2,
                    $f2,
                    14,
                    15
                ),

Small snippet after:

                0b00 => blend4!(
                    $a, $b, $c, $d, $e, $f, 6, 7, $a2, $b2, $c2, $d2, $e2,
                    $f2, 14, 15,
                ),

Looks like that trailing comma might be at fault. In particular, blend4! is:

macro_rules! blend4 {
        (
            $a:expr,
            $b:expr,
            $c:expr,
            $d:expr,
            $e:expr,
            $f:expr,
            $g:expr,
            $h:expr,
            $i:expr,
            $j:expr,
            $k:expr,
            $l:expr,
            $m:expr,
            $n:expr,
            $o:expr,
            $p:expr
        ) => {
            simd_shuffle16(
                a,
                b,
                [
                    $a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k, $l, $m,
                    $n, $o, $p,
                ],
            )
        };
    }

Code before:

/// Blend packed 16-bit integers from `a` and `b` using control mask `imm8`.
#[inline]
#[target_feature(enable = "avx2")]
#[cfg_attr(test, assert_instr(vpblendw, imm8 = 9))]
#[rustc_args_required_const(2)]
pub unsafe fn _mm256_blend_epi16(
    a: __m256i, b: __m256i, imm8: i32
) -> __m256i {
    let imm8 = (imm8 & 0xFF) as u8;
    let a = a.as_i16x16();
    let b = b.as_i16x16();
    macro_rules! blend4 {
        (
            $a:expr,
            $b:expr,
            $c:expr,
            $d:expr,
            $e:expr,
            $f:expr,
            $g:expr,
            $h:expr,
            $i:expr,
            $j:expr,
            $k:expr,
            $l:expr,
            $m:expr,
            $n:expr,
            $o:expr,
            $p:expr
        ) => {
            simd_shuffle16(
                a,
                b,
                [
                    $a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k, $l, $m,
                    $n, $o, $p,
                ],
            )
        };
    }
    macro_rules! blend3 {
        (
            $a:expr,
            $b:expr,
            $c:expr,
            $d:expr,
            $e:expr,
            $f:expr,
            $a2:expr,
            $b2:expr,
            $c2:expr,
            $d2:expr,
            $e2:expr,
            $f2:expr
        ) => {
            match (imm8 >> 6) & 0b11 {
                0b00 => blend4!(
                    $a,
                    $b,
                    $c,
                    $d,
                    $e,
                    $f,
                    6,
                    7,
                    $a2,
                    $b2,
                    $c2,
                    $d2,
                    $e2,
                    $f2,
                    14,
                    15
                ),
                0b01 => blend4!(
                    $a,
                    $b,
                    $c,
                    $d,
                    $e,
                    $f,
                    22,
                    7,
                    $a2,
                    $b2,
                    $c2,
                    $d2,
                    $e2,
                    $f2,
                    30,
                    15
                ),
                0b10 => blend4!(
                    $a,
                    $b,
                    $c,
                    $d,
                    $e,
                    $f,
                    6,
                    23,
                    $a2,
                    $b2,
                    $c2,
                    $d2,
                    $e2,
                    $f2,
                    14,
                    31
                ),
                _ => blend4!(
                    $a,
                    $b,
                    $c,
                    $d,
                    $e,
                    $f,
                    22,
                    23,
                    $a2,
                    $b2,
                    $c2,
                    $d2,
                    $e2,
                    $f2,
                    30,
                    31
                ),
            }
        };
    }
    macro_rules! blend2 {
        (
            $a:expr,
            $b:expr,
            $c:expr,
            $d:expr,
            $a2:expr,
            $b2:expr,
            $c2:expr,
            $d2:expr
        ) => {
            match (imm8 >> 4) & 0b11 {
                0b00 => blend3!(
                    $a,
                    $b,
                    $c,
                    $d,
                    4,
                    5,
                    $a2,
                    $b2,
                    $c2,
                    $d2,
                    12,
                    13
                ),
                0b01 => blend3!(
                    $a,
                    $b,
                    $c,
                    $d,
                    20,
                    5,
                    $a2,
                    $b2,
                    $c2,
                    $d2,
                    28,
                    13
                ),
                0b10 => blend3!(
                    $a,
                    $b,
                    $c,
                    $d,
                    4,
                    21,
                    $a2,
                    $b2,
                    $c2,
                    $d2,
                    12,
                    29
                ),
                _ => blend3!(
                    $a,
                    $b,
                    $c,
                    $d,
                    20,
                    21,
                    $a2,
                    $b2,
                    $c2,
                    $d2,
                    28,
                    29
                ),
            }
        };
    }
    macro_rules! blend1 {
        ($a1:expr, $b1:expr, $a2:expr, $b2:expr) => {
            match (imm8 >> 2) & 0b11 {
                0b00 => blend2!($a1, $b1, 2, 3, $a2, $b2, 10, 11),
                0b01 => blend2!($a1, $b1, 18, 3, $a2, $b2, 26, 11),
                0b10 => blend2!($a1, $b1, 2, 19, $a2, $b2, 10, 27),
                _ => blend2!($a1, $b1, 18, 19, $a2, $b2, 26, 27),
            }
        };
    }
    let r: i16x16 = match imm8 & 0b11 {
        0b00 => blend1!(0, 1, 8, 9),
        0b01 => blend1!(16, 1, 24, 9),
        0b10 => blend1!(0, 17, 8, 25),
        _ => blend1!(16, 17, 24, 25),
    };
    mem::transmute(r)
}

Code after:

/// Blend packed 16-bit integers from `a` and `b` using control mask `imm8`.
#[inline]
#[target_feature(enable = "avx2")]
#[cfg_attr(test, assert_instr(vpblendw, imm8 = 9))]
#[rustc_args_required_const(2)]
pub unsafe fn _mm256_blend_epi16(
    a: __m256i, b: __m256i, imm8: i32,
) -> __m256i {
    let imm8 = (imm8 & 0xFF) as u8;
    let a = a.as_i16x16();
    let b = b.as_i16x16();
    macro_rules! blend4 {
        (
            $a:expr,
            $b:expr,
            $c:expr,
            $d:expr,
            $e:expr,
            $f:expr,
            $g:expr,
            $h:expr,
            $i:expr,
            $j:expr,
            $k:expr,
            $l:expr,
            $m:expr,
            $n:expr,
            $o:expr,
            $p:expr
        ) => {
            simd_shuffle16(
                a,
                b,
                [
                    $a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k, $l, $m,
                    $n, $o, $p,
                ],
            )
        };
    }
    macro_rules! blend3 {
        (
            $a:expr,
            $b:expr,
            $c:expr,
            $d:expr,
            $e:expr,
            $f:expr,
            $a2:expr,
            $b2:expr,
            $c2:expr,
            $d2:expr,
            $e2:expr,
            $f2:expr
        ) => {
            match (imm8 >> 6) & 0b11 {
                0b00 => blend4!(
                    $a, $b, $c, $d, $e, $f, 6, 7, $a2, $b2, $c2, $d2, $e2,
                    $f2, 14, 15,
                ),
                0b01 => blend4!(
                    $a, $b, $c, $d, $e, $f, 22, 7, $a2, $b2, $c2, $d2,
                    $e2, $f2, 30, 15,
                ),
                0b10 => blend4!(
                    $a, $b, $c, $d, $e, $f, 6, 23, $a2, $b2, $c2, $d2,
                    $e2, $f2, 14, 31,
                ),
                _ => blend4!(
                    $a, $b, $c, $d, $e, $f, 22, 23, $a2, $b2, $c2, $d2,
                    $e2, $f2, 30, 31,
                ),
            }
        };
    }
    macro_rules! blend2 {
        (
            $a:expr,
            $b:expr,
            $c:expr,
            $d:expr,
            $a2:expr,
            $b2:expr,
            $c2:expr,
            $d2:expr
        ) => {
            match (imm8 >> 4) & 0b11 {
                0b00 => blend3!(
                    $a, $b, $c, $d, 4, 5, $a2, $b2, $c2, $d2, 12, 13
                ),
                0b01 => blend3!(
                    $a, $b, $c, $d, 20, 5, $a2, $b2, $c2, $d2, 28, 13
                ),
                0b10 => blend3!(
                    $a, $b, $c, $d, 4, 21, $a2, $b2, $c2, $d2, 12, 29
                ),
                _ => blend3!(
                    $a, $b, $c, $d, 20, 21, $a2, $b2, $c2, $d2, 28, 29
                ),
            }
        };
    }
    macro_rules! blend1 {
        ($a1:expr, $b1:expr, $a2:expr, $b2:expr) => {
            match (imm8 >> 2) & 0b11 {
                0b00 => blend2!($a1, $b1, 2, 3, $a2, $b2, 10, 11),
                0b01 => blend2!($a1, $b1, 18, 3, $a2, $b2, 26, 11),
                0b10 => blend2!($a1, $b1, 2, 19, $a2, $b2, 10, 27),
                _ => blend2!($a1, $b1, 18, 19, $a2, $b2, 26, 27),
            }
        };
    }
    let r: i16x16 = match imm8 & 0b11 {
        0b00 => blend1!(0, 1, 8, 9),
        0b01 => blend1!(16, 1, 24, 9),
        0b10 => blend1!(0, 17, 8, 25),
        _ => blend1!(16, 17, 24, 25),
    };
    mem::transmute(r)
}
@nrc nrc added bug Panic, non-idempotency, invalid code, etc. a-macros labels Apr 11, 2018
@nrc
Copy link
Member

nrc commented Apr 11, 2018

Are you using the latest version of rustfmt? We've fixed this bug in the past (several times), it's possible this is a new instance, but could just be out of date

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Apr 11, 2018

So I just did an update and the current rustfmt introduces the issue as well. I think last time I updated was 1-2 days ago - I would say yesterday but I can't tell for sure, I should have written the version before updating... In any case, that version also had the bug.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Apr 11, 2018

We are running rustfmt from rustup nightly on CI, and it has this issue as well: https://travis-ci.org/gnzlbg/stdsimd/jobs/365024247#L2052 (notice the added trailing comma on that line).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-macros bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Development

No branches or pull requests

2 participants