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

Better SIMD shuffles #36624

Open
jneem opened this Issue Sep 21, 2016 · 2 comments

Comments

Projects
None yet
4 participants
@jneem
Copy link
Contributor

jneem commented Sep 21, 2016

I'm trying to do the following in AVX2 using intrinsics: shift x one byte to the right, while shifting the rightmost byte of y into the leftmost byte of x. This is best done using two instructions: vperm2i128 followed by vpalignr. However, simd_shuffle32 generates four instructions: vmovdqa (to load a constant), vpblendvb, then vperm2i128 and vpalignr. Here is a a full example, which may be compiled with rustc -O -C target_feature=+avx2 --crate-type=lib --emit=asm shuffle.rs.

#![feature(platform_intrinsics, repr_simd)]

#[allow(non_camel_case_types)]
#[repr(simd)]
pub struct u8x32(u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8);

extern "platform-intrinsic" {
    fn simd_shuffle32<T, U>(x: T, y: T, idx: [u32; 32]) -> U;
}

pub fn right_shift_1(left: u8x32, right: u8x32) -> u8x32 {
    unsafe { simd_shuffle32(left, right, [31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62]) }
}

This might be considered a bug in LLVM, in the sense that it's generating a sub-optimal shuffle. However, I think it should be addressed in rustc, because if I know what the right sequence of instructions is then I shouldn't have to hope that LLVM can generate it. Moreover, it's possible to get the right code from clang (compile with clang -emit-llvm -mavx2 -O -S shuffle.c):

#include <immintrin.h>
__m256i right_shift_1(__m256i left, __m256i right)
{
    __m256i new_left = _mm256_permute2x128_si256(left, right, 33);
    return _mm256_alignr_epi8(new_left, right, 1);
}

A possibly interesting observation is that the unoptimized LLVM IR from clang contains a llvm.x86.avx2.vperm2i128 intrinsic followed by a shufflevector. The optimized LLVM IR from clang contains two shufflevector intrinsics. In order to try to get the same output from rustc, I first patched it to support llvm.x86.avx2.vperm2i128. After modifying right_shift_1 to use the new intrinsic, I got rustc to produce llvm.x86.avx2.vperm2i128 followed by a shufflevector. However, the optimized LLVM IR from rustc still produces a single shufflevector, and it still ends up producing the bad asm.

I think this means that the fault is from some optimization pass in rustc that isn't in clang, but I haven't had time to investigate it yet...

jneem referenced this issue in rust-lang/regex Mar 13, 2018

teddy: port teddy searcher to AVX2
This commit adds a copy of the Teddy searcher that works on AVX2. We
don't attempt to reuse any code between them just yet, and instead just
copy & paste and tweak parts of it to work on 32 bytes instead of 16.
(Some parts were trickier than others. For example, @jneem figured out
how to nearly compensate for the lack of a real 256-bit bytewise PALIGNR
instruction, which we borrow here.)

Overall, AVX2 provides a nice bump in performance.
@BurntSushi

This comment has been minimized.

Copy link
Member

BurntSushi commented Mar 13, 2018

This still occurs even when using the specific intrinsics in std::arch, which is somewhat surprising! See: rust-lang/regex@f962ddb#r28069091

@gnzlbg

This comment has been minimized.

Copy link
Contributor

gnzlbg commented Mar 27, 2018

Can reproduce, will try to fill in an LLVM bug for this.

Once the LLVM bug is filled and recognized as a real bug we could workaround this in std::arch (rustc is definitely the wrong place to do it).

EDIT: reported https://bugs.llvm.org/show_bug.cgi?id=36933

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.