On Thu, May 3, 2018 at 4:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
>> Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>>> Isn't there a hidden assumption about endianness there?
Right, thanks.
>> Yeah, I was wondering about that too, but does anyone actually run
>> ARMs big-endian?
I think all the distros I follow dropped big endian support in recent
years, but that's no excuse.
> After a bit more thought ... we could remove the magic constant,
> along with any assumptions about endianness, by doing it like this:
>
> result = (pg_comp_crc32c_armv8(0, &data, sizeof(data)) ==
> pg_comp_crc32c_sb8(0, &data, sizeof(data)));
>
> Of course we'd eat a few more cycles during the test, but it's hard
> to see that mattering.
I was thinking of proposing this:
- uint64 data = 42;
+ uint64 data = UINT64CONST(0x4242424242424242);
... and:
- result = (pg_comp_crc32c_armv8(0, &data, sizeof(data))
== 0xdd439b0d);
+ result = (pg_comp_crc32c_armv8(0, &data, sizeof(data))
== 0x54eb145b);
No strong preference though.
> It strikes me also that, at least for debugging purposes, it's seriously
> awful that you can't tell from outside what result this function got.
> Certainly the outcome that "pg_comp_crc32c_armv8 executed but returned
> the wrong answer" is not something that ought to go unremarked.
> We could elog the results, but I'm not very sure what log level is
> appropriate --- also, I doubt we want another log entry from every
> launched process, so how to prevent that?
I don't think *broken* CPUs are something we need to handle, are they?
Hmm, I suppose there might be a hypothetical operating system or ARM
chip that somehow doesn't raise SIGILL and returns garbage, and then
it's nice to fall back to the software version (unless you're 1/2^32
unlucky).
For debugging I was just putting a temporary elog in. Leaving one in
there at one of the DEBUG levels seems reasonable though.
--
Thomas Munro
http://www.enterprisedb.com
