I wrote:
> It seems there are two problems:
> 1. pg_cpu.h believes that x86-specific code can be conditional on
> #if defined(USE_SSE2) || defined(__i386__)
> but macOS doesn't define __i386__, only __x86_64__. It works
> anyway on single-arch builds because the test to set USE_SSE2
> succeeds, but not on multi-arch builds.
> 2. checksum.c believes that it's okay to call x86_feature_available
> if USE_AVX2_WITH_RUNTIME_CHECK is set.
After further study, I found that USE_SSE2 is set like this in c.h:
#if (defined(__x86_64__) || defined(_M_AMD64))
#define USE_SSE2
...
So my interpretation of what was happening in pg_cpu.h was wrong.
I think we ought to s/USE_SSE2/__x86_64__/ there just to make this
less confusing, but the part of my quick-hack patch that touched
pg_cpu.h was really a no-op.
Also, the reason we manage to set USE_AVX2_WITH_RUNTIME_CHECK
in a multi-arch build is that what configure actually tests
is (a) is $host_cpu x86_64 and (b) does the compiler accept
__attribute__((target("avx2"))). On a multi-arch build,
the ARM side spits out a warning
'+avx2' is not a recognized feature for this target (ignoring feature)
but it doesn't actually fail, so configure thinks that's a success.
I think that a cleaner solution to this is to fix pg_cpu.h so
that x86_feature_available() exists but returns constant false
on non-x86 platforms. A proposed patch for that is attached.
If we just do that much, a multi-arch macOS build produces the "not a
recognized feature" warning while compiling checksum.c, but it works.
Given that the build also produces all those ranlib warnings, I don't
know that suppressing "not a recognized feature" is worth worrying
about. But if we want to, something similar to the checksum.c change
I posted before would do it.
regards, tom lane
PS: does c.h's check for _M_AMD64 really do anything? If we need
it there, why not in all the other places that check __x86_64__?
diff --git a/src/include/port/pg_cpu.h b/src/include/port/pg_cpu.h
index 566ed7a16e3..5b25747e43a 100644
--- a/src/include/port/pg_cpu.h
+++ b/src/include/port/pg_cpu.h
@@ -13,8 +13,6 @@
#ifndef PG_CPU_H
#define PG_CPU_H
-#if defined(USE_SSE2) || defined(__i386__)
-
typedef enum X86FeatureId
{
/* Have we run feature detection? */
@@ -43,6 +41,8 @@ typedef enum X86FeatureId
} X86FeatureId;
#define X86FeaturesSize (PG_TSC_ADJUST + 1)
+#if defined(__x86_64__) || defined(__i386__)
+
extern PGDLLIMPORT bool X86Features[];
extern void set_x86_features(void);
@@ -58,6 +58,14 @@ x86_feature_available(X86FeatureId feature)
extern uint32 x86_tsc_frequency_khz(char *source, size_t source_size);
-#endif /* defined(USE_SSE2) || defined(__i386__) */
+#else /* !(defined(__x86_64__)||defined(__i386__)) */
+
+static inline bool
+x86_feature_available(X86FeatureId feature)
+{
+ return false;
+}
+
+#endif /* defined(__x86_64__) || defined(__i386__) */
#endif /* PG_CPU_H */
diff --git a/src/port/pg_cpu_x86.c b/src/port/pg_cpu_x86.c
index 0405ba19f6f..b050677f717 100644
--- a/src/port/pg_cpu_x86.c
+++ b/src/port/pg_cpu_x86.c
@@ -19,7 +19,7 @@
#include "postgres_fe.h"
#endif
-#if defined(USE_SSE2) || defined(__i386__)
+#if defined(__x86_64__) || defined(__i386__)
#ifdef _MSC_VER
#include <intrin.h>
@@ -287,10 +287,10 @@ x86_hypervisor_tsc_frequency_khz(void)
return 0;
}
-#else /* defined(USE_SSE2) || defined(__i386__) */
+#else /* defined(__x86_64__) || defined(__i386__) */
/* prevent linker complaints about empty module */
extern int pg_cpu_x86_dummy_variable;
int pg_cpu_x86_dummy_variable = 0;
-#endif /* ! (USE_SSE2 || __i386__) */
+#endif /* ! (__x86_64__ || __i386__) */
