allow unaligned reads with _mm_loadl_epi64

[brian-armstrong]

As discussed in #582 it is "safe" to read unaligned memory locations with _mm_loadl_epi64. This code was suggested by scottmcm in the Rust Discord - I'm not familiar enough with this part of Rust to offer much commentary on it.

But to note, I have tried this with my code that uses this intrinsic and am seeing the behavior I'd expect to see.

[gnzlbg]

The type annotation here (::<__m128i>) shouldn't be necessary.

Maybe one can just use _mm_loadu_si128 here ?

[brian-armstrong]

Depends - in my case, I need to do a shuffle afterwards anyway, so it comes out to be about the same. If you just wanted to load the lower 64 bits though, then adding in an unnecessary shuffle afterwards would likely cost some performance. At any rate, the fact that the instruction, when seemingly used correctly, yields a segfault is fairly surprising.

[gnzlbg]

The ptr::read_unaligned reads 128bits, and so does _mm_loadu_si128, or what am I missing? With a shuffle for the first 64 bits afterwards, LLVM should optimize this to a 64-bit load in both cases.

[brian-armstrong]

Oh, yeah, you're right. I actually don't think I know how to write this PR, now that I think about it.

[gnzlbg]

You are doing great! I think you can replace the ptr::read_unaligned(mem_addr) with just _mm_loadu_si128(mem_addr) and that should just work.

[gnzlbg]

Thanks for the PR. There is only one nitpick, otherwise this LGTM.

[brian-armstrong]

So I cleaned up my example and got the right flags so that godbolt shows what I think is the right instructions. I'm not sure how to translate this into Rust though.

https://gcc.godbolt.org/z/EZv0qv

Notably,

_mm_loadl_epi64(...);
_mm_storel_epi64(...);

becomes

        mov     rax, qword ptr [rdi + 24]
        mov     qword ptr [rdi + 40], rax

It's weird that it takes a qword ptr because I think it really only is loading a dword here, though I guess I have nothing to base that on.

[gnzlbg]

It's weird that it takes a qword ptr because I think it really only is loading a dword here, though I guess I have nothing to base that on.

So the Intrinsics Guide says these should lower to a movq, but these intrinsics are so simple, that the code that they will lower to will heavily depend on what the code at the call site around them is doing because LLVM can easily reason about them.

When we merge this, you can try them in your app, and if you see unexpected code generation we can revise their implementation.

[gnzlbg]

Thanks!

[brian-armstrong]

So I'm looking at the trace in my actual program, and I believe the C version which uses _mm_loadl_epi64 is emitting movq while Rust (with this patch) is emitting vmovqdu. It's unclear if this is because llvm is unable to optimize the Rust version for some reason or if the C version has just correctly determined that only a quadword is needed, not a double quadword.

[brian-armstrong]

I wonder if the right code here isn't just

    _mm_set_epi64x(0, ptr::read_unaligned(mem_addr as *const i64))

[gnzlbg]

That could work too. Could you modify your stdsimd copy locally, and use it in your project, and report whether that works better?

[brian-armstrong]

Yes, it does emit movq. It's hard to say whether that's generally faster, but it does seem closer to the original spirit of the intrinsic.