Re: Not-terribly-safe checks for CRC intrinsic support

From: Steven Niu <niushiji(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Not-terribly-safe checks for CRC intrinsic support
Date: 2025-03-17 01:43:26
Message-ID: f22358cb-3e32-4967-b2e1-f500f7150864@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


+# is missing, we must link not just compile, and store the results in
global

The "compile" should be "compiler"?

Regards,
Steven

在 2025/3/15 7:04, Tom Lane 写道:
> 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
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2025-03-17 01:56:19 Re: Not-terribly-safe checks for CRC intrinsic support
Previous Message Hayato Kuroda (Fujitsu) 2025-03-17 01:22:59 RE: long-standing data loss bug in initial sync of logical replication