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
From 7b7d8bc1aa04ee7d8d2d98b9af90c28dc15de580 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 29 Jul 2025 14:29:40 +0900
Subject: [PATCH v2] Fix ./configure checks with __cpuidex() and __cpuid()

---
 configure    | 81 +++++++++++++++++++++++++++-------------------------
 configure.ac | 49 ++++++++++++++++---------------
 meson.build  |  5 +---
 3 files changed, 69 insertions(+), 66 deletions(-)

diff --git a/configure b/configure
index 6d7c22e153fe..dd1b177ba1ce 100755
--- a/configure
+++ b/configure
@@ -17565,7 +17565,7 @@ $as_echo "#define HAVE_GCC__ATOMIC_INT64_CAS 1" >>confdefs.h
 fi
 
 
-# Check for x86 cpuid instruction
+# Check for __get_cpuid() and __cpuid()
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __get_cpuid" >&5
 $as_echo_n "checking for __get_cpuid... " >&6; }
 if ${pgac_cv__get_cpuid+:} false; then :
@@ -17598,8 +17598,44 @@ if test x"$pgac_cv__get_cpuid" = x"yes"; then
 
 $as_echo "#define HAVE__GET_CPUID 1" >>confdefs.h
 
+else
+  # __cpuid()
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __cpuid" >&5
+$as_echo_n "checking for __cpuid... " >&6; }
+if ${pgac_cv__cpuid+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <intrin.h>
+int
+main ()
+{
+unsigned int exx[4] = {0, 0, 0, 0};
+    __cpuid(exx, 1);
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  pgac_cv__cpuid="yes"
+else
+  pgac_cv__cpuid="no"
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__cpuid" >&5
+$as_echo "$pgac_cv__cpuid" >&6; }
+  if test x"$pgac_cv__cpuid" = x"yes"; then
+
+$as_echo "#define HAVE__CPUID 1" >>confdefs.h
+
+  fi
 fi
 
+# Check for __get_cpuid_count() and __cpuidex() in a similar fashion.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __get_cpuid_count" >&5
 $as_echo_n "checking for __get_cpuid_count... " >&6; }
 if ${pgac_cv__get_cpuid_count+:} false; then :
@@ -17632,43 +17668,9 @@ if test x"$pgac_cv__get_cpuid_count" = x"yes"; then
 
 $as_echo "#define HAVE__GET_CPUID_COUNT 1" >>confdefs.h
 
-fi
-
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __cpuid" >&5
-$as_echo_n "checking for __cpuid... " >&6; }
-if ${pgac_cv__cpuid+:} false; then :
-  $as_echo_n "(cached) " >&6
 else
-  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h.  */
-#include <intrin.h>
-int
-main ()
-{
-unsigned int exx[4] = {0, 0, 0, 0};
-  __get_cpuid(exx[0], 1);
-
-  ;
-  return 0;
-}
-_ACEOF
-if ac_fn_c_try_link "$LINENO"; then :
-  pgac_cv__cpuid="yes"
-else
-  pgac_cv__cpuid="no"
-fi
-rm -f core conftest.err conftest.$ac_objext \
-    conftest$ac_exeext conftest.$ac_ext
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__cpuid" >&5
-$as_echo "$pgac_cv__cpuid" >&6; }
-if test x"$pgac_cv__cpuid" = x"yes"; then
-
-$as_echo "#define HAVE__CPUID 1" >>confdefs.h
-
-fi
-
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __cpuidex" >&5
+  # __cpuidex()
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __cpuidex" >&5
 $as_echo_n "checking for __cpuidex... " >&6; }
 if ${pgac_cv__cpuidex+:} false; then :
   $as_echo_n "(cached) " >&6
@@ -17680,7 +17682,7 @@ int
 main ()
 {
 unsigned int exx[4] = {0, 0, 0, 0};
-  __get_cpuidex(exx[0], 7, 0);
+    __cpuidex(exx, 7, 0);
 
   ;
   return 0;
@@ -17696,10 +17698,11 @@ rm -f core conftest.err conftest.$ac_objext \
 fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__cpuidex" >&5
 $as_echo "$pgac_cv__cpuidex" >&6; }
-if test x"$pgac_cv__cpuidex" = x"yes"; then
+  if test x"$pgac_cv__cpuidex" = x"yes"; then
 
 $as_echo "#define HAVE__CPUIDEX 1" >>confdefs.h
 
+  fi
 fi
 
 # Check for XSAVE intrinsics
diff --git a/configure.ac b/configure.ac
index c2877e369350..7eff51f38812 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2044,7 +2044,7 @@ PGAC_HAVE_GCC__ATOMIC_INT32_CAS
 PGAC_HAVE_GCC__ATOMIC_INT64_CAS
 
 
-# Check for x86 cpuid instruction
+# Check for __get_cpuid() and __cpuid()
 AC_CACHE_CHECK([for __get_cpuid], [pgac_cv__get_cpuid],
 [AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <cpuid.h>],
   [[unsigned int exx[4] = {0, 0, 0, 0};
@@ -2054,8 +2054,21 @@ AC_CACHE_CHECK([for __get_cpuid], [pgac_cv__get_cpuid],
   [pgac_cv__get_cpuid="no"])])
 if test x"$pgac_cv__get_cpuid" = x"yes"; then
   AC_DEFINE(HAVE__GET_CPUID, 1, [Define to 1 if you have __get_cpuid.])
+else
+  # __cpuid()
+  AC_CACHE_CHECK([for __cpuid], [pgac_cv__cpuid],
+  [AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <intrin.h>],
+    [[unsigned int exx[4] = {0, 0, 0, 0};
+    __cpuid(exx, 1);
+    ]])],
+    [pgac_cv__cpuid="yes"],
+    [pgac_cv__cpuid="no"])])
+  if test x"$pgac_cv__cpuid" = x"yes"; then
+    AC_DEFINE(HAVE__CPUID, 1, [Define to 1 if you have __cpuid.])
+  fi
 fi
 
+# Check for __get_cpuid_count() and __cpuidex() in a similar fashion.
 AC_CACHE_CHECK([for __get_cpuid_count], [pgac_cv__get_cpuid_count],
 [AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <cpuid.h>],
   [[unsigned int exx[4] = {0, 0, 0, 0};
@@ -2065,28 +2078,18 @@ AC_CACHE_CHECK([for __get_cpuid_count], [pgac_cv__get_cpuid_count],
   [pgac_cv__get_cpuid_count="no"])])
 if test x"$pgac_cv__get_cpuid_count" = x"yes"; then
   AC_DEFINE(HAVE__GET_CPUID_COUNT, 1, [Define to 1 if you have __get_cpuid_count.])
-fi
-
-AC_CACHE_CHECK([for __cpuid], [pgac_cv__cpuid],
-[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <intrin.h>],
-  [[unsigned int exx[4] = {0, 0, 0, 0};
-  __get_cpuid(exx[0], 1);
-  ]])],
-  [pgac_cv__cpuid="yes"],
-  [pgac_cv__cpuid="no"])])
-if test x"$pgac_cv__cpuid" = x"yes"; then
-  AC_DEFINE(HAVE__CPUID, 1, [Define to 1 if you have __cpuid.])
-fi
-
-AC_CACHE_CHECK([for __cpuidex], [pgac_cv__cpuidex],
-[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <intrin.h>],
-  [[unsigned int exx[4] = {0, 0, 0, 0};
-  __get_cpuidex(exx[0], 7, 0);
-  ]])],
-  [pgac_cv__cpuidex="yes"],
-  [pgac_cv__cpuidex="no"])])
-if test x"$pgac_cv__cpuidex" = x"yes"; then
-  AC_DEFINE(HAVE__CPUIDEX, 1, [Define to 1 if you have __cpuidex.])
+else
+  # __cpuidex()
+  AC_CACHE_CHECK([for __cpuidex], [pgac_cv__cpuidex],
+  [AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <intrin.h>],
+    [[unsigned int exx[4] = {0, 0, 0, 0};
+    __cpuidex(exx, 7, 0);
+    ]])],
+    [pgac_cv__cpuidex="yes"],
+    [pgac_cv__cpuidex="no"])])
+  if test x"$pgac_cv__cpuidex" = x"yes"; then
+    AC_DEFINE(HAVE__CPUIDEX, 1, [Define to 1 if you have __cpuidex.])
+  fi
 fi
 
 # Check for XSAVE intrinsics
diff --git a/meson.build b/meson.build
index 5365aaf95e64..ca423dc8e12f 100644
--- a/meson.build
+++ b/meson.build
@@ -1996,10 +1996,7 @@ if cc.links('''
   cdata.set('HAVE__BUILTIN_OP_OVERFLOW', 1)
 endif
 
-
-# XXX: The configure.ac check for __cpuid() is broken, we don't copy that
-# here. To prevent problems due to two detection methods working, stop
-# checking after one.
+# Check for __get_cpuid() and __cpuid().
 if cc.links('''
     #include <cpuid.h>
     int main(int arg, char **argv)
-- 
2.50.0

Attachment: signature.asc
Description: PGP signature

Reply via email to