Not-terribly-safe checks for CRC intrinsic support

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Not-terribly-safe checks for CRC intrinsic support
Date: 2025-03-14 23:04:22
Message-ID: 3368102.1741993462@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I noticed that our configuration-time checks for the presence
of CRC intrinsics generally look like

unsigned int crc = 0;
crc = __crc32cb(crc, 0);
crc = __crc32ch(crc, 0);
crc = __crc32cw(crc, 0);
crc = __crc32cd(crc, 0);
/* return computed value, to prevent the above being optimized away */
return crc == 0;

The trouble with this is that "crc" is a local variable, so the
compiler would be perfectly within its rights to optimize the whole
thing down to "return some_constant". While that outcome sufficiently
proves that the compiler has heard of these intrinsics, it fails to
prove that the platform has any necessary library infrastructure,
assembler support for the opcodes, etc etc. Whoever originally
wrote this evidently had concern for that hazard, or they'd not
have bothered with forcing a dependency on the final value; but
that seems insufficient. We have other nearby tests that try
to avoid this problem by making the functions-under-test operate
on global variables, so I think we should do likewise here.

In connection with bug #18839[1], I checked to see if this might
already be happening. At least with gcc 12.2 on armhf Debian,
it doesn't seem to: the compiler still generates the crc opcodes.
But the same compiler is perfectly willing to optimize a call to
sin(3) down to a constant under similar conditions. So I think this
is just a matter of they didn't get round to it, not that there's a
principled reason to think they won't ever get round to it. There
might be other cases where these probes are already missing something,
and we've not noticed because there's-compiler-support-but-no-
library-support is surely a very rare case in the field.

In short, I think we ought to apply and perhaps back-patch something
like the attached.

BTW, it looks to me like PGAC_AVX512_POPCNT_INTRINSICS is at similar
hazard, but I'm not entirely sure how to fix that one.

Thoughts?

regards, tom lane

[1] https://www.postgresql.org/message-id/18839-7615d0f8267dc015%40postgresql.org

Attachment Content-Type Size
be-more-paranoid-in-CRC-configure-checks.patch text/x-diff 7.4 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message David G. Johnston 2025-03-15 00:42:29 Re: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.
Previous Message David E. Wheeler 2025-03-14 21:46:54 Re: PG_CFLAGS rpath Passthrough Issue