On Thu, Feb 27, 2025 at 8:26 AM Dave Hansen <dave.hansen@xxxxxxxxx> wrote:
> On 2/26/25 22:12, Bill Wendling wrote:
> > #ifdef CONFIG_X86_64
> > -#define CRC32_INST "crc32q %1, %q0"
> > +#define CRC32_INST __builtin_ia32_crc32di
> > #else
> > -#define CRC32_INST "crc32l %1, %0"
> > +#define CRC32_INST __builtin_ia32_crc32si
> > #endif
> >
> > /*
> > @@ -78,10 +78,10 @@ u32 crc32c_le_arch(u32 crc, const u8 *p, size_t len)
> >
> > for (num_longs = len / sizeof(unsigned long);
> > num_longs != 0; num_longs--, p += sizeof(unsigned long))
> > - asm(CRC32_INST : "+r" (crc) : "rm" (*(unsigned long *)p));
> > + crc = CRC32_INST(crc, *(unsigned long *)p);
>
> Could we get rid of the macros, please?
>
> unsigned long crc32_ul(unsigned long crc, unsigned long data)
> {
> if (IS_DEFINED(CONFIG_X86_64))
> return __builtin_ia32_crc32di(crc, data)
> else
> return __builtin_ia32_crc32si(crc, data)
> }
>
> I guess it could also do some check like:
>
> if (sizeof(int) == sizeof(long))
>
> instead of CONFIG_X86_64, but the CONFIG_X86_64 will make it more
> obvious when someone comes through to rip out 32-bit support some day.
I vastly prefer the first way if made "static __always_inline".
-bw
