At 2014-12-30 16:05:50 +0200, hlinnakan...@vmware.com wrote:
>
> A couple of quick comments:
>
> bswap32 is unused on on little-endian systems. That will give a
> compiler warning.
Huh. I don't get a warning, even when I add -Wunused to the build flags.
But since you mention it, it would be better to write the function thus:
static inline uint32 cpu_to_le32(uint32 x)
{
#ifndef WORDS_BIGENDIAN
return x;
#elif defined(__GNUC__) || defined(__clang__)
return __builtin_bswap32(x);
#else
return ((x << 24) & 0xff000000) |
((x << 8) & 0x00ff0000) |
((x >> 8) & 0x0000ff00) |
((x >> 24) & 0x000000ff);
#endif
}
> pg_comp_crc32c_sse […] fetches the 8-byte chunks from only 4-byte
> aligned addresses. Is that intentional?
Thanks for spotting that. I had meant to change the test to "& 7". But
again, now that you mention it, I'm not sure it's necessary. The CRC32*
instructions don't have the usual SSE alignment requirements, and I see
that the Linux kernel (among other implementations) process eight bytes
at a time from the start of the buffer and then process the remaining
bytes one at a time. I'll do a bit more research and post an update.
Thanks for having a look.
-- Abhijit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
