RE: Improve CRC32C performance on SSE4.2
Devulapalli, Raghuveer <raghuveer.devulapalli@intel.com>
From: "Devulapalli, Raghuveer" <raghuveer.devulapalli@intel.com>
To: John Naylor <johncnaylorls@gmail.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "Shankaran, Akash" <akash.shankaran@intel.com>
Date: 2025-03-03T22:40:27Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Include _mm512_zextsi128_si512() in AVX-512 configure probes.
- ccd5bc93fdfe 18.0 landed
-
Properly fix AVX-512 CRC calculation bug
- 43da394304fb 18.0 landed
-
Workaround code generation bug in clang
- f83f14881c7a 18.0 landed
-
Compute CRC32C using AVX-512 instructions where available
- 3c6e8c123896 18.0 landed
-
Inline CRC computation for small fixed-length input on x86
- e2809e3a1015 18.0 landed
-
Be more paranoid in configure's checks for CRC and POPCNT intrinsics.
- fdb5dd6331e3 18.0 cited
Hi John,
> You raised some interesting points, which deserve a thoughtful response. After
> sleeping on it, however I came to the conclusion that a sweeping change in
> runtime checks, with either of our approaches, has downsides and unresolved
> questions. Perhaps we can come back to it at a later time.
Sounds good to me. I did get derailed into something beyond the scope of this patch. I will make a separate proposal once this is merged.
> Here's what I came up with in v11:
Some feedback on v11:
if ((exx[2] & (1 << 20)) != 0) /* SSE 4.2 */
{
pg_comp_crc32c = pg_comp_crc32c_sse42;
#ifdef USE_PCLMUL_WITH_RUNTIME_CHECK
if ((exx[2] & (1 << 1)) != 0) /* PCLMUL */
pg_comp_crc32c = pg_comp_crc32c_pclmul;
#endif
}
#ifdef USE_SSE42_CRC32C_WITH_RUNTIME_CHECK
else
pg_comp_crc32c = pg_comp_crc32c_sb8;
#endif
Is the #ifdef USE_SSE42_CRC32C_WITH_RUNTIME_CHECK at the right place? Shouldn’t it guard SSE4.2 function pointer assignment?
/* WIP: configure checks */
#ifdef __x86_64__
#define USE_PCLMUL_WITH_RUNTIME_CHECK
#endif
Minor consideration: gcc 4.3 (released in 2011) is the only compiler that supports -msse4.2 and not -mpclmul. gcc >= 4.4 supports both. If you are okay causing a regression on gcc4.3, we could combine USE_PCLMUL_WITH_RUNTIME_CHECK with USE_SSE42_CRC32C_WITH_RUNTIME_CHECK into a single macro to reduce the number of #ifdef's in the codebase and simplify configure/meson compiler checks.
> 0001: same benchmark module as before
> 0002: For SSE4.2 builds, arrange so that constant input uses an inlined path so
> that the compiler can emit unrolled loops anywhere.
When building with meson, it looks like we build with -O2 and that is not enough for the compiler to unroll the SSE42 CRC32C loop. It requires -O3 or -O2 with -funroll-loops (see https://gcc.godbolt.org/z/4Eaq981aT). Perhaps we should check disassembly to see if the unroll is really happening on constant input?
Also, the reason you have pg_comp_crc32c_sse42_inline defined separately in a header file is because you want to (a) inline the function and (b) unroll for constant inputs. Couldn't both of these be achieved by adding function __attribute__((always_inline)) on the pg_comp_crc32c_sse42 function with the added -funroll-loops compiler flag?
Raghuveer