Re: [PATCH] crc: Add PCLMUL implementation

[Sam Russell]

I've added an alignment check in lib/crc, it looks like the code works okay without it for me but an _m128 is supposed to be 128-bit aligned so I'm happy that I've added it.

The attached patch renames the module to crc-x86_64 while keeping the source file crc-x86_64-pclmul.c, as well as the alignment check above.

test-crc and bench-crc are fine, gzip builds with this gnulib and decompresses my test file with no hash error

On Tue, 26 Nov 2024 at 20:29, Sam Russell <sam.h.russell@gmail.com> wrote:

> Cool. But it even gets better: one can use these target options on a per-
>   function basis, via __attribute__. See
>   https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/x86-Function-Attributes.html#index-target_0028_0022avx_0022_0029-function-attribute_002c-x86
>   https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/x86-Function-Attributes.html#index-target_0028_0022pclmul_0022_0029-function-attribute_002c-x86
> 
>   With this, you are not forced to make the pclmul code a separate compilation
>   unit. You can rename the .c file to a .h file and #include it from crc.h.
>   (If you like. It's a matter of style at this point.)

> Don't need three different modules, right? A package will either want or
> not want x86_64 specific optimizations. Why would a package need to say
> "I want this optimization but not the other ones"?

I'd prefer to keep them as separate files if you're okay with it. I did a quick experiment and by wrapping each function in push_options and pop_options pragmas it was pretty easy to get it all working in the same file so I can confirm that option does work. It makes sense to keep them in the same module though, I agree. Ultimately we can delay this decision until we have the avx2/avx512 implementations, I'll rename the module to crc-x86_64 in the meantime though.

0001-crc-Add-PCLMUL-implementation.patch
Description: Binary data