simd-json icon indicating copy to clipboard operation
simd-json copied to clipboard

Fix x86 compilation

Open PigeonF opened this issue 1 year ago • 3 comments

Compiling for the i686-unknown-linux-gnu target leads to multiple compilation failures.

The first set was easy to solve (akin to #246). The second issue is that std::arch::x86_64::_mm_cvtsi128_si64 does not seem to have an equivalent for std::arch::x86. I think this is an oversight in std::arch::x86?

I am not confident enough in my SIMD knowledge to create a clever workaround, so I copied the "native rust implementation" for x86. If there is some workaround without using _mm_cvtsi128_si64, then feel free to change it.

[!NOTE] I do not have a x86 system myself, so I could not test the changes locally. I only checked by running cargo build --target i686-unknown-linux-gnu

PigeonF avatar Jun 26 '24 14:06 PigeonF

nice catch and good solution. Using the native implementation is fine, 32-bit x86 systems are rare these days so I don't think it'll have a wide impact and with LLVM's optimisations it'll be plenty fast.

I suspect that the reason _mm_cvtsi128_si64 doesn't exist in x86 is that it returns a 64 bit value and the x86 cpu's don't have registers to fit that into nicely.

The arch alias is a nice addition too, I never thought of that but it's much cleaner, I learned some new trick here :) thanks!

There are a few issues with the tests since we got a new clippy lints, those are not related to your changes I think. I'm happy to fix those in the main branch and rebase your PR tomorrow if you don't want to deal with them.

CI aside, PR looks great 👍 thanks!

Licenser avatar Jun 26 '24 17:06 Licenser

There is one more clippy lint that is reported on nightly: missing_transmute_annotations, specifically for

https://github.com/simd-lite/simd-json/blob/83a806f82155ed177807fa0af3e597737cd71a2d/src/macros.rs#L1253-L1259

I am not sure if you want to just ignore the lint here or not, so I have not done anything about them. Properly fixing the lint would probably involve removing the macros and calling transmute directly instead, which seems like a whole lot of effort. On the other hand, I am not familiar with the code base, so I am not comfortable deciding that the lint should be ignored 😅

PigeonF avatar Jun 26 '24 18:06 PigeonF

Perfect :) thank you so much!

Licenser avatar Jun 27 '24 08:06 Licenser

Sorry it took me so long to merge this, I thought I had done it and must have forgotten.

Licenser avatar Jul 04 '24 17:07 Licenser