Re: Optimize Arm64 crc32c implementation in Postgresql

On Fri, Mar 2, 2018 at 10:36 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2018-01-10 05:58:19 +0000, Yuqi Gu wrote:
>> +#ifdef USE_ARMCE_CRC32C_WITH_RUNTIME_CHECK
>> +#include <sys/auxv.h>
>> +#include <asm/hwcap.h>
>> +#ifndef HWCAP_CRC32
>> +#define HWCAP_CRC32 (1 << 7)
>> +#endif
>
>> +static bool
>> +pg_crc32c_arm64ce_available(void) {
>> + unsigned long auxv = getauxval(AT_HWCAP);
>> + return (auxv & HWCAP_CRC32) != 0;
>> +}
>> +
>> +#else
>
> What's the availability of these headers and functions on non-linux platforms?

FWIW I don't think that'll work on FreeBSD. I don't have an arm64
system to test on right now, but I can see that there is no
getauxval() like glibc's. FreeBSD *might* provide the same sort of
information via procstat_getauxv() from libprocstat, but I think maybe
not because I don't see any trace of HWCAP_CRC32 in the tree and I see
a different approach to testing the CPU ID registers in eg
libkern/crc32.c.

So... that stuff probably needs either a configure check for the
getauxval function and/or those headers, or an OS check?

While I'm looking at this:

-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))

Why? Doesn't something << 62 have the same value and type as
(something << 31) << 31?

+ --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run]

What is this for?

+ if (length & sizeof(uint16)) {
+ CRC32CH(crc32_c, *(uint16*)p_buf);
+ p_buf += sizeof(uint16);
+ }
+
+ if (length & sizeof(uint8)) {
+ CRC32CB(crc32_c, *p_buf);
+ }

From the department of trivialities, our coding style has braces like this:

if (length & sizeof(uint16))
{
CRC32CH(crc32_c, *(uint16*)p_buf);
p_buf += sizeof(uint16);
}

if (length & sizeof(uint8))
CRC32CB(crc32_c, *p_buf);

--
Thomas Munro
http://www.enterprisedb.com