Re: [PATCH] crc: New optimised slice-by-8 implementation

[Sam Russell]

Tests pass with GL_CRC_SLICE_BY_8 set

On Wed, 16 Oct 2024 at 13:52, Simon Josefsson <simon@josefsson.org> wrote:

Sam Russell <sam.h.russell@gmail.com> writes:

>> People using the crc module who want to disable the faster/larger CRC32
>> implementation can add 'gl_crc_slice_by_8=yes' to their configure.ac
>> before invoking gnulib.
>
> I've been running tests with `./gnulib-tool --with-tests --test crc` so I'm
> interested in a way of enabling this flag from gnulib-tool

You could add a module like 'crc-slice-by-8' that depend on 'crc' with a
'configure.ac-early' statement that does 'gl_crc_slice_by_8=yes'.

Maybe there are cleaner/simpler solutions.

>> If code is copied then licensing terms has to be clear.  RFCs have bad
>> licensing terms, so maybe we cannot use this code at all.
>
> The existing lib/crc.c uses code directly taken from RFC 1952 and is
> referenced as such, I sent off my copyright paperwork and mentioned this to
> them and am awaiting a reply.

Nice catch, we should re-evaluate what code we can use and how.  The
snippet that is copied seems fairly small.

>> That memcpy looks weird, does it really do what you want it to do on all
>> architectures?  Would using something from endian.h be cleaner?
>
> The purpose is to work around alignment issues. The signature
> for crc32_update_no_xor() takes a char* and casting up to a uint64_t is
> undefined by C. Using memcpy lets the compiler and runtime be smart and
> insulates us from nonaligned memory accesses. I am open to other ideas
> around this (we can step forward byte by byte until we reach an aligned
> pointer and then do 8-byte blocks from there, for example, but then we need
> to detect alignment and do the upcasting ourselves)
>
> You are correct that this breaks under big endian though, thanks for the
> tip for endian.h. I've added a le64toh() call and this should make it work
> correctly on big endian systems (how do we go about testing these btw? I
> don't have one on me).

The GCC compile farm has plenty of different architectures, so if you
write some low-level code that may have endian issues, testing for that
is a good idea.  You can test it on your own machine too, using QEMU.

/Simon

0001-crc-New-optimised-slice-by-8-implementation.patch
Description: Binary data