Re: Broken ./configure checks for __cpuid() and __cpuidex()
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Lukas Fittl <lukas@fittl.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-07-29T05:30:06Z
Lists: pgsql-hackers
Attachments
- v2-0001-Fix-.-configure-checks-with-__cpuidex-and-__cpuid.patch (text/x-diff) patch v2-0001
On Mon, Jul 28, 2025 at 08:53:43PM -0700, Lukas Fittl wrote: > Could it be that the problem only happens when including both cpuid.h and > intrin.h, because they both define __cpuid? (the configure check only > includes intrin.h) > > My theory when I worked on the patch that Michael referenced in the > original email was that intrin.h is only for MSVC (for GCC at least, > __cpuidex is defined in cpuid.h). Ah, yes, you have the right point here, and that would be obviously the right way to do it and also why I guess MinGW is not complaining with meson: meson.build does not check for the second pieces if the first checks pass. configure checks each of these four APIs individually, and all of them detected in MinGW. So we can simply mimick in ./configure what meson does like in the attached, which has been working for some time now. The CI is happy with this version for me, with MSVC reporting the second steps, and MinGW reporting the first steps. That would be for the buildfarm to decide if it is entirely stable, but from my perspective I am pretty confident that this patch should be OK as-is. And that's pretty much what the CRC32 code expects from these checks: we only want one of these routines. > I'm not sure how to get CI to run MinGW (it appears paused for me?), so I > can't test this myself easily. src/tools/ci/README, "Enabling cirrus-ci in a github repository". I've enabled it in my own copy of Postgres on github, relying on that as an extra pre-commit check mostly for patches that are OS-sensitive. It runs independently on the CI, relying on the OS base images that Andres has been cooking for the last few years, of course. > But the relevant change would be to change "defined(HAVE__CPUIDEX)" to > "(defined(HAVE__CPUIDEX) && defined(_MSC_VER))" for the guard on both > intrin.h includes. _MSC_VER is a flag specific to MSVC, so we'd still get the problem with MinGW, no? So we'd still have the same problem. Perhaps we'll need the _MSC_VER piece for your other patch, but for the sake of the back-branches and what we are doing here it does not seem necessary to me to do so. -- Michael
Commits
-
Fix ./configure checks with __cpuidex() and __cpuid()
- 612f5b80620d 13.22 landed
- 60953d4cba7c 14.19 landed
- d6ffc43f95b2 15.14 landed
- c1984be23cdc 16.10 landed
- 8de56323c139 17.6 landed
- cd2d52cc6b39 18.0 landed
- 1a5212775e46 19 (unreleased) landed