pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedDetection of hadware feature => please do not use signal
37+ messages / 5 participants
[nested] [flat]
* Detection of hadware feature => please do not use signal
@ 2024-10-31 15:41 Bastien Roucariès <rouca@debian.org>
0 siblings, 1 reply; 37+ messages in thread
From: Bastien Roucariès @ 2024-10-31 15:41 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
Hi,
On debian side we will like to avoid use of sigill to detect feature.
https://sources.debian.org/src/postgresql-17/17.0-1/src/port/pg_crc32c_armv8_choose.c/#L55
is really bad.
Could you use the canonical way under linux
#include <sys/auxv.h>
#include <asm/hwcap.h>
#if defined(__aarch64__)
return !(getauxval(AT_HWCAP) & HWCAP_CRC32);
#else
return !(getauxval(AT_HWCAP2) & HWCAP2_CRC32);
#endif
Thanks
Rouca
Attachments:
[application/pgp-signature] signature.asc (832B, ../../4496616.iHFcN1HehY@portable-bastien/2-signature.asc)
download
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-10-31 17:29 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Bastien Roucariès <rouca@debian.org>
0 siblings, 2 replies; 37+ messages in thread
From: Heikki Linnakangas @ 2024-10-31 17:29 UTC (permalink / raw)
To: Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On 31/10/2024 17:41, Bastien Roucariès wrote:
> Hi,
>
> On debian side we will like to avoid use of sigill to detect feature.
>
> https://sources.debian.org/src/postgresql-17/17.0-1/src/port/pg_crc32c_armv8_choose.c/#L55
>
> is really bad.
Why is it bad?
> Could you use the canonical way under linux
>
> #include <sys/auxv.h>
> #include <asm/hwcap.h>
>
> #if defined(__aarch64__)
> return !(getauxval(AT_HWCAP) & HWCAP_CRC32);
> #else
> return !(getauxval(AT_HWCAP2) & HWCAP2_CRC32);
> #endif
We used to do that, but it was changed in commit 1c72ec6f. Looking at
the discussion that led to it, it's not clear to me why we switched [1].
Portablity to non-glibc systems maybe?
If there's a handy, portable way to check CPU capabilities, I agree it
seem less hacky than catching SIGILL...
[1]
https://www.postgresql.org/message-id/CAEepm%3D02Run-Pk3xyt%2BRV3p1N%2B7cKZxN95_MamaJw8Cnw%2BDwjQ%40...
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-10-31 17:42 Bastien Roucariès <rouca@debian.org>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
1 sibling, 1 reply; 37+ messages in thread
From: Bastien Roucariès @ 2024-10-31 17:42 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
Le jeudi 31 octobre 2024, 17:29:45 UTC Heikki Linnakangas a écrit :
> On 31/10/2024 17:41, Bastien Roucariès wrote:
> > Hi,
> >
> > On debian side we will like to avoid use of sigill to detect feature.
> >
> > https://sources.debian.org/src/postgresql-17/17.0-1/src/port/pg_crc32c_armv8_choose.c/#L55
> >
> > is really bad.
>
> Why is it bad?
for a binary it is maybe safe for a library it is unsafe in Multi thread context to capture signal...
And we are burned by a recent CVE in ssh due to signal handling.
>
> > Could you use the canonical way under linux
> >
> > #include <sys/auxv.h>
> > #include <asm/hwcap.h>
> >
> > #if defined(__aarch64__)
> > return !(getauxval(AT_HWCAP) & HWCAP_CRC32);
> > #else
> > return !(getauxval(AT_HWCAP2) & HWCAP2_CRC32);
> > #endif
>
> We used to do that, but it was changed in commit 1c72ec6f. Looking at
> the discussion that led to it, it's not clear to me why we switched [1].
> Portablity to non-glibc systems maybe?
Yes I suppose but a
#ifdef __GLIBC__
#else
#endif
Will be safe
>
> If there's a handy, portable way to check CPU capabilities, I agree it
> seem less hacky than catching SIGILL...
unfortunatly no
>
> [1]
> https://www.postgresql.org/message-id/CAEepm%3D02Run-Pk3xyt%2BRV3p1N%2B7cKZxN95_MamaJw8Cnw%2BDwjQ%40...
>
>
Attachments:
[application/pgp-signature] signature.asc (832B, ../../193820057.q8YS0ZdlI3@portable-bastien/2-signature.asc)
download
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-10-31 17:48 Tom Lane <tgl@sss.pgh.pa.us>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
1 sibling, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-10-31 17:48 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 31/10/2024 17:41, Bastien Roucariès wrote:
>> Could you use the canonical way under linux
> We used to do that, but it was changed in commit 1c72ec6f. Looking at
> the discussion that led to it, it's not clear to me why we switched [1].
The complaint was precisely that it would not work under non-linux.
Maybe we could do it one way on linux and the other way everywhere
else, but I do not find that attractive; it just makes it harder
to get full test coverage.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-10-31 18:01 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 37+ messages in thread
From: Heikki Linnakangas @ 2024-10-31 18:01 UTC (permalink / raw)
To: Bastien Roucariès <rouca@debian.org>; +Cc: pgsql-bugs@lists.postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>
On 31/10/2024 19:48, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> On 31/10/2024 17:41, Bastien Roucariès wrote:
>>> Could you use the canonical way under linux
>
>> We used to do that, but it was changed in commit 1c72ec6f. Looking at
>> the discussion that led to it, it's not clear to me why we switched [1].
>
> The complaint was precisely that it would not work under non-linux.
>
> Maybe we could do it one way on linux and the other way everywhere
> else, but I do not find that attractive; it just makes it harder
> to get full test coverage.
Bastien, would you happen to know how other portable libraries that need
to run on other operating system do this? We added the SIGILL probe back
in 2018, there might be better options available now.
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-10-31 18:25 Tom Lane <tgl@sss.pgh.pa.us>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 2 replies; 37+ messages in thread
From: Tom Lane @ 2024-10-31 18:25 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 31/10/2024 19:48, Tom Lane wrote:
>> Maybe we could do it one way on linux and the other way everywhere
>> else, but I do not find that attractive; it just makes it harder
>> to get full test coverage.
> Bastien, would you happen to know how other portable libraries that need
> to run on other operating system do this? We added the SIGILL probe back
> in 2018, there might be better options available now.
It occurs to me to wonder whether the existing code works on Windows.
Windows-on-ARM wasn't a thing we thought about in 2018, but it's
a reasonable target now.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-10-31 20:12 Bastien Roucariès <rouca@debian.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 37+ messages in thread
From: Bastien Roucariès @ 2024-10-31 20:12 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
Le jeudi 31 octobre 2024, 18:25:34 UTC Tom Lane a écrit :
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
> > On 31/10/2024 19:48, Tom Lane wrote:
> >> Maybe we could do it one way on linux and the other way everywhere
> >> else, but I do not find that attractive; it just makes it harder
> >> to get full test coverage.
>
> > Bastien, would you happen to know how other portable libraries that need
> > to run on other operating system do this? We added the SIGILL probe back
> > in 2018, there might be better options available now.
>
> It occurs to me to wonder whether the existing code works on Windows.
> Windows-on-ARM wasn't a thing we thought about in 2018, but it's
> a reasonable target now.
openssl use the following
https://github.com/openssl/openssl/blob/59f5f6c73cd2e1e2bd8ef405fdb6fadf0711f639/crypto/armcap.c#L31...
# if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
# if __GLIBC_PREREQ(2, 16)
# include <sys/auxv.h>
# define OSSL_IMPLEMENT_GETAUXVAL
# endif
# elif defined(__ANDROID_API__)
/* see https://developer.android.google.cn/ndk/guides/cpu-features */
# if __ANDROID_API__ >= 18
# include <sys/auxv.h>
# define OSSL_IMPLEMENT_GETAUXVAL
# endif
# endif
# if defined(__FreeBSD__) || defined(__OpenBSD__)
# include <sys/param.h>
# if (defined(__FreeBSD__) && __FreeBSD_version >= 1200000) || \
(defined(__OpenBSD__) && OpenBSD >= 202409)
# include <sys/auxv.h>
# define OSSL_IMPLEMENT_GETAUXVAL
static unsigned long getauxval(unsigned long key)
{
unsigned long val = 0ul;
if (elf_aux_info((int)key, &val, sizeof(val)) != 0)
return 0ul;
return val;
}
# endif
# endif
/*
* Android: according to https://developer.android.com/ndk/guides/cpu-features,
* getauxval is supported starting with API level 18
*/
# if defined(__ANDROID__) && defined(__ANDROID_API__) && __ANDROID_API__ >= 18
# include <sys/auxv.h>
# define OSSL_IMPLEMENT_GETAUXVAL
# endif
/*
* ARM puts the feature bits for Crypto Extensions in AT_HWCAP2, whereas
* AArch64 used AT_HWCAP.
*/
# ifndef AT_HWCAP
# define AT_HWCAP 16
# endif
# ifndef AT_HWCAP2
# define AT_HWCAP2 26
# endif
# if defined(__arm__) || defined (__arm)
# define OSSL_HWCAP AT_HWCAP
# define OSSL_HWCAP_NEON (1 << 12)
# define OSSL_HWCAP_CE AT_HWCAP2
# define OSSL_HWCAP_CE_AES (1 << 0)
# define OSSL_HWCAP_CE_PMULL (1 << 1)
# define OSSL_HWCAP_CE_SHA1 (1 << 2)
# define OSSL_HWCAP_CE_SHA256 (1 << 3)
# elif defined(__aarch64__)
# define OSSL_HWCAP AT_HWCAP
# define OSSL_HWCAP_NEON (1 << 1)
# define OSSL_HWCAP_CE AT_HWCAP
# define OSSL_HWCAP_CE_AES (1 << 3)
# define OSSL_HWCAP_CE_PMULL (1 << 4)
# define OSSL_HWCAP_CE_SHA1 (1 << 5)
# define OSSL_HWCAP_CE_SHA256 (1 << 6)
# define OSSL_HWCAP_CPUID (1 << 11)
# define OSSL_HWCAP_SHA3 (1 << 17)
# define OSSL_HWCAP_CE_SM3 (1 << 18)
# define OSSL_HWCAP_CE_SM4 (1 << 19)
# define OSSL_HWCAP_CE_SHA512 (1 << 21)
# define OSSL_HWCAP_SVE (1 << 22)
/* AT_HWCAP2 */
# define OSSL_HWCAP2 26
# define OSSL_HWCAP2_SVE2 (1 << 1)
# define OSSL_HWCAP2_RNG (1 << 16)
# endif
For windows
https://sources.debian.org/src/xz-utils/5.6.3-1/src/liblzma/check/crc32_arm64.h/?hl=82#L92
>
> regards, tom lane
>
Attachments:
[application/pgp-signature] signature.asc (832B, ../../5114283.1BQ3OT5r56@portable-bastien/2-signature.asc)
download
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-10-31 20:16 Tom Lane <tgl@sss.pgh.pa.us>
parent: Bastien Roucariès <rouca@debian.org>
0 siblings, 0 replies; 37+ messages in thread
From: Tom Lane @ 2024-10-31 20:16 UTC (permalink / raw)
To: Bastien Roucariès <rouca@debian.org>; +Cc: pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
Bastien =?ISO-8859-1?Q?Roucari=E8s?= <rouca@debian.org> writes:
> Le jeudi 31 octobre 2024, 17:29:45 UTC Heikki Linnakangas a écrit :
>> Why is it bad?
> for a binary it is maybe safe for a library it is unsafe in Multi thread context to capture signal...
[ shrug... ] If we're concerned about that, we should hardly be less
concerned on other platforms than we are on Linux.
(AFAIK, we do not execute the code in question in any libraries.)
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-01 04:28 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 2 replies; 37+ messages in thread
From: Thomas Munro @ 2024-11-01 04:28 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Fri, Nov 1, 2024 at 7:25 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> It occurs to me to wonder whether the existing code works on Windows.
> Windows-on-ARM wasn't a thing we thought about in 2018, but it's
> a reasonable target now.
I looked into that[1] and decided that I was going to ignore it
completely, because:
* Windows defines a dummy SIGILL signal number as required by the C
standard, so stuff like this compiles, but
EXCEPTION_ILLEGAL_INSTRUCTION isn't connected up to it and you'd just
crash instead, but even if it were...
* we don't install a native signal handler anyway, just one of our
fake ones (that I would love to get rid of)
* there is also a native API to check CPU features, but I don't see
the point in even thinking about it for now, because...
* Windows 11 effectively requires ARMv8.1-A to boot, and that's what
comes installed on current machines you can buy (this is expressed
differently officially, by saying that LSE atomics are required and by
listing the supported CPUs by model not ISA, and the list of CPUs that
are no longer supported can also be found online, so it's fairly
clear; there are also fun investigative reports from the time when
RPI4s and other never-officially-supported systems could suddenly no
longer boot after some update, as they would croak on LSE
instructions)
* if someone is using Windows 10 for ARM (which I gather is getting
harder to obtain by now) on an old enough low power laptop that lacks
the instruction (and note that most ARMv8-A chips *did* have that
instruction as an optional extra anyway, they just didn't have LSE),
well then maybe it's an issue but I think for a new platform where we
are so short of developers that we haven't even got past other really
basic starter problems after a couple of years of talking about it, I
think we should focus only on current systems instead of allowing even
1 nanosecond to be wasted on retro-computing topics... and in any case
that hypothetical user is just about out of time because...
* Windows 10's announced EOL (no more updates/support, full screen
warning messages if past EOLs are anything to go by) coincides
approximately with PostgreSQL 18's expected release
(Huh, thinking about other synchronous signals, don't we also fail to
install a native SIGFPE handler on Windows, that would convert eg
div-by-zero in C code into an ereport on Unix?)
Anyway I don't mind getting rid of the SIGILL stuff as long as the new
coding is tidy and cross-platform enough, if people are insisting.
I'm not aware that there's anything technically wrong with it on any
Unix system, but I agree that it's not beautiful code. FreeBSD was my
motivation at the time, but it has since gained elf_aux_info(), and
OpenBSD too, and I can help write/test that part if we go that way.
Or maybe we could just take some fragments of OpenSSL or libzlma or
whatever code if the licensing aspect is OK.
As for the other ARM platforms in our universe:
Unfortunately it looks like NetBSD doesn't put AT_HWCAP into its
auxv[], or even expose it to user space nicely, and those libraries
don't seem to know of another way. Hopefully NetBSD will align with
those other systems for portability's sake. The only other way I
could find in a quick googling session is /usr/sbin/cpuctl, root only,
no cigar (but a solid clue that the information is floating around
somewhere, it just depends where exactly the root-only fencing is
happening). Even if a way can be found, it'd be better to be able to
do it the same way as other systems that we are actually testing if at
all possible. Put that way, the build farm absence is a sort of vote
for just not even trying to detect it on NetBSD. A NetBSD user who
really wants to access the feature could still compile with
-march=<new thing>, supply a build farm animal and a patch, or
preferrably help get a compatible elf_auxv_info(AT_HWCAP) merged into
NetBSD. That'd be my vote if we switch to auxv probing on the other
systems.
Macs don't do ELF or auxv, but there's a sysctl. I think it must
always report true in practice. The first M1 was ARMv8.4-A. (There
are old non-computer Apple devices that used ARMv8-A still in the
wild, which is interesting to me only because it explains the recent
invention of -moutline-atomics, about which more soon hopefully, to
get faster lwlocks etc on our -march=armv8-a builds as shipped by
Debian et al.) So I think we could skip the palaver and just
hard-code the knowledge that Macs can do this stuff.
(Archeological note: the reason several systems have the same auxv[]
concept, ie a table of parameters that exec*() sets up alongside
argv[] and environ[] to communicate stuff about memory layout etc
primarily to libc, and the reason they even agree on some of the
parameter names, is that it came from SVR4 with ELF. I think Sun's
original version got AT_HWCAP from the ELF binary after selecting from
several object variants that were compiled to match various CPU
feature sets, as a way of shipping fat binaries that go faster on the
right hardware, while these modern systems are just passing on
whatever the CPU reports under the same hijacked name; it's related,
but different... Anyway they needed some way for the kernel to give
the features to user space on ARM, because its register that is
equivalent to x86's CPUID can't be accessed from user space's
privilege level and libc itself would like to be able to use some
fancy features. Even for non-ARM architectures it's nice not to have
to break out the assembler in user programs that want to do the same
sorts of tricks. Amazingly, illumos can apparently run on ARM now so
we might in theory encounter the old meaning of AT_HWCAP, but that's
quite a hypothetical unicorn so I'm filing the thought under
archeology and hiding it in parentheses.)
Note that Andres recently wondered out loud[2] if CRC32 might be
fundamentally the wrong tool for the job in a related thread, so
perhaps this will all become moot if someone does the research and
replaces it, but that's vapourware for now...
[1] https://www.postgresql.org/message-id/CA%2BhUKGJ2B5rAGUncAob%3DChutCT%3Dfx0Ot7kwvio5cB7NpOGKG1Q%40ma...
[2] https://www.postgresql.org/message-id/flat/20240612193746.rjeiip4hcamjedgo%40awork3.anarazel.de#ab38...
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-01 04:44 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
1 sibling, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-01 04:44 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
Thomas Munro <thomas.munro@gmail.com> writes:
> On Fri, Nov 1, 2024 at 7:25 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It occurs to me to wonder whether the existing code works on Windows.
>> Windows-on-ARM wasn't a thing we thought about in 2018, but it's
>> a reasonable target now.
> I looked into that[1] and decided that I was going to ignore it
> completely, because:
> [ oodles o' details ]
Hmm. So it seems like we could do this:
* Linux, FreeBSD, OpenBSD: do run-time test as recommended
* Windows, macOS: assume that supported ARM hardware can do this
* anything else: assume no hardware CRC
The only thing I'd be even a bit sad about there is not having
NetBSD support. Maybe we just need to research that a bit more,
or maybe we have to wait for/lobby for them to do what the other
BSDen have done. But in any case I'm not seeing NetBSD-on-ARM
as a platform that's important enough to block movement on this,
let alone anything else in the "anything else" category.
An alternative to "assume no hardware CRC" could be
* anything else: use the existing SIGILL test
But I do sympathize with Bastien's concerns about that.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-01 06:16 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 37+ messages in thread
From: Tom Lane @ 2024-11-01 06:16 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
I wrote:
> Hmm. So it seems like we could do this:
> * Linux, FreeBSD, OpenBSD: do run-time test as recommended
> * Windows, macOS: assume that supported ARM hardware can do this
> * anything else: assume no hardware CRC
Actually, after chewing on that second point awhile longer,
how about this modest proposal:
* Drop all code for run-time determination of ARM CRC support.
Assume it's there unless user builds with a -march option that
says it definitely isn't.
Realistically, exactly who is going to be running Postgres 18+
on ARM hardware that lacks CRC support? I can think of lots of
projects that are more worthy of our time than this.
(Perhaps it's time to apply the same mindset to x86 CRC support
too?)
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-01 10:20 Bastien Roucariès <rouca@debian.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 37+ messages in thread
From: Bastien Roucariès @ 2024-11-01 10:20 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
Le vendredi 1 novembre 2024, 06:16:32 UTC Tom Lane a écrit :
> I wrote:
> > Hmm. So it seems like we could do this:
> > * Linux, FreeBSD, OpenBSD: do run-time test as recommended
> > * Windows, macOS: assume that supported ARM hardware can do this
> > * anything else: assume no hardware CRC
>
> Actually, after chewing on that second point awhile longer,
> how about this modest proposal:
>
> * Drop all code for run-time determination of ARM CRC support.
> Assume it's there unless user builds with a -march option that
> says it definitely isn't.
>
> Realistically, exactly who is going to be running Postgres 18+
> on ARM hardware that lacks CRC support? I can think of lots of
> projects that are more worthy of our time than this.
>
> (Perhaps it's time to apply the same mindset to x86 CRC support
> too?)
arm64 baseline for debian is without CRC so I will prefer to keep crc32
Bastien
>
> regards, tom lane
>
Attachments:
[application/pgp-signature] signature.asc (832B, ../../7915750.ZPPAYPQ7GT@portable-bastien/2-signature.asc)
download
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-02 05:11 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 2 replies; 37+ messages in thread
From: Thomas Munro @ 2024-11-02 05:11 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Fri, Nov 1, 2024 at 7:16 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
> > Hmm. So it seems like we could do this:
> > * Linux, FreeBSD, OpenBSD: do run-time test as recommended
> > * Windows, macOS: assume that supported ARM hardware can do this
> > * anything else: assume no hardware CRC
>
> Actually, after chewing on that second point awhile longer,
> how about this modest proposal:
>
> * Drop all code for run-time determination of ARM CRC support.
> Assume it's there unless user builds with a -march option that
> says it definitely isn't.
For CRC32, that would work (and already does via configure tests) for
macOS (I'd forgotten about that in my earlier message, Macs never even
compile the CRC32 "choose" code because the system compiler targets M1
by default), recent RHEL (see below), and presumably others, but it
would penalise Debian, FreeBSD (using the standard binary package
repo) and others if we didn't have the runtime check due to their
conservative choice of baseline arch (as Bastien just said about
Debian in a crossed email). I've been wondering about what to do
about all this for a while... There are also other features we aren't
using yet, but should, or single instructions that we are calling
through function pointers, preventing automatic vectorisation etc.
> Realistically, exactly who is going to be running Postgres 18+
> on ARM hardware that lacks CRC support? I can think of lots of
> projects that are more worthy of our time than this.
>
> (Perhaps it's time to apply the same mindset to x86 CRC support
> too?)
I think it's quite interesting that RHEL has moved its baseline to
x86_86-v2, having recently worked with Intel and AMD to standardise
new levels "-vN" that group and "linearise" the feature sets, and
others were already talking about standarding on -v3 last time I
looked, while Debian still targets x86_86 which means the instruction
set of the first AMD K8 chip from 2003. Debian has a nice wiki page
(see thread ↓) explaining various workaround strategies, ranging from
runtime tests like ours, per-sub-architecture library selection,
through to (last resort) depending on pseudo-packages like
"x86-64-v2-support" so that your package becomes uninstallable on
ancient hardware. I'm not involved in packaging but I was wondering
out loud whether the PGDG repos might potentially consider making a
different choice than the official Debian repos, always or as an
option, or ... various other ideas proposed by others. Perhaps we
could continue the topic on that thread:
https://www.postgresql.org/message-id/flat/CA%2BhUKGKS64zJezV9y9mPcB-J0i%2BfLGiv3FAdwSH_3SCaVdrjyQ%4...
In the meantime I'll see about the Linux/*BSD patch for this thing,
more tomorrow hopefully. And I might see if I can find a NetBSD
person to ask...
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-07 00:01 Thomas Munro <thomas.munro@gmail.com>
parent: Thomas Munro <thomas.munro@gmail.com>
1 sibling, 1 reply; 37+ messages in thread
From: Thomas Munro @ 2024-11-07 00:01 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Sat, Nov 2, 2024 at 6:11 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> In the meantime I'll see about the Linux/*BSD patch for this thing,
Here is a patch, tested on Debian 12 and FreeBSD 15 and observed to
return true here as expected. It might be hard to find a machine
where it returns false. From some quick googling, only the first
generation of 64 bit designs eg Cortex A53 lacked CRC32, so I tried
booting in qemu with -cpu cortex-a53 but alas its emulation appears to
be inaccurate there (both software emulation and hypervisor, features
as reported in dmesg) and unlike other fancy instructions qemu lacks
an off switch for that specific feature. I guess someone who wants to
see false there would need to compile on an old Chromebook or similar!
It doesn't look like I need to include <asm/hwcap.h> from Bastien's
original message, because <sys/auxv.h> pulls in <bits/hwcap.h>
already. The man pages are pretty useless on this topic, but I'd
rather avoid more configure tests if possible and FreeBSD has those
constants in <machine/elf.h>, which it also pulls in already with just
#include <sys/auxv.h>. I don't have an OpenBSD/aarch64 setup to test
(neither does our build farm) but I'm pretty sure from browsing their
code on github that this'll just work; it has the same <sys/auxv.h>
and <machine/elf.h> setup and all the same names for everything.
Attachments:
[text/x-patch] 0001-Use-auxv-to-check-for-CRC32-instructions-on-ARM.patch (5.9K, ../../CA+hUKGLKjuZACHB559EzFFgzGzk20Qr-S5fqg4C-F7sAg4q6+g@mail.gmail.com/2-0001-Use-auxv-to-check-for-CRC32-instructions-on-ARM.patch)
download | inline diff:
From 871dd92dccc4ba90977c2af647d103d50a9e5319 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Wed, 6 Nov 2024 22:05:13 +0000
Subject: [PATCH] Use auxv to check for CRC32 instructions on ARM.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously we probed for CRC32 instructions by testing if they raised
SIGILL. Some have expressed doubts about that technique, and it's not
exactly beautiful. Now that several operating systems expose the
contents of the feature register to user space (something like x86's
CPUID, but not accessible without kernel help on ARM), let's use that
instead.
This is expected to work on Linux, FreeBSD and OpenBSD.
For macOS we don't use this code at all, because its compiler already
assumes a higher -march level by default and our configure tests detect
that and use CRC32 builtins directly without probing first; therefore we
don't bother to implement the macOS way of probing.
For NetBSD, CRC32 instruction probing is lost with this commit when
using a conservative -march setting. Patches are welcome, but if NetBSD
ever adopts the same API as FreeBSD and OpenBSD this change should
hopefully just work there too.
Suggested-by: Bastien Roucariès <rouca@debian.org>
Discussion: https://postgr.es/m/4496616.iHFcN1HehY%40portable-bastien
---
configure | 2 +-
configure.ac | 2 ++
meson.build | 2 ++
src/include/pg_config.h.in | 6 ++++
src/port/pg_crc32c_armv8_choose.c | 54 +++++++------------------------
5 files changed, 22 insertions(+), 44 deletions(-)
diff --git a/configure b/configure
index 6e256b417b9..1e4c6f3d5cf 100755
--- a/configure
+++ b/configure
@@ -15044,7 +15044,7 @@ fi
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-for ac_func in backtrace_symbols copyfile copy_file_range getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l
+for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info getauxval getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.ac b/configure.ac
index 3992694dacc..1bc19af7dff 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1717,6 +1717,8 @@ AC_CHECK_FUNCS(m4_normalize([
backtrace_symbols
copyfile
copy_file_range
+ elf_aux_info
+ getauxval
getifaddrs
getpeerucred
inet_pton
diff --git a/meson.build b/meson.build
index 9a98f0c86a0..d3ac84d72fd 100644
--- a/meson.build
+++ b/meson.build
@@ -2628,7 +2628,9 @@ func_checks = [
# when enabling asan the dlopen check doesn't notice that -ldl is actually
# required. Just checking for dlsym() ought to suffice.
['dlsym', {'dependencies': [dl_dep], 'define': false}],
+ ['elf_aux_info'],
['explicit_bzero'],
+ ['getauxval'],
['getifaddrs'],
['getopt', {'dependencies': [getopt_dep, gnugetopt_dep], 'skip': always_replace_getopt}],
['getopt_long', {'dependencies': [getopt_dep, gnugetopt_dep], 'skip': always_replace_getopt_long}],
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index cdd9a6e9355..a903c60a3a0 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -125,6 +125,9 @@
/* Define to 1 if you have the <editline/readline.h> header file. */
#undef HAVE_EDITLINE_READLINE_H
+/* Define to 1 if you have the `elf_aux_info' function. */
+#undef HAVE_ELF_AUX_INFO
+
/* Define to 1 if you have the <execinfo.h> header file. */
#undef HAVE_EXECINFO_H
@@ -154,6 +157,9 @@
*/
#undef HAVE_GCC__SYNC_INT64_CAS
+/* Define to 1 if you have the `getauxval' function. */
+#undef HAVE_GETAUXVAL
+
/* Define to 1 if you have the `getifaddrs' function. */
#undef HAVE_GETIFADDRS
diff --git a/src/port/pg_crc32c_armv8_choose.c b/src/port/pg_crc32c_armv8_choose.c
index eaf39cf3e81..fff926b13a7 100644
--- a/src/port/pg_crc32c_armv8_choose.c
+++ b/src/port/pg_crc32c_armv8_choose.c
@@ -24,57 +24,25 @@
#include "postgres_fe.h"
#endif
-#include <setjmp.h>
-#include <signal.h>
+#if defined(HAVE_ELF_AUX_INFO) || defined(HAVE_GETAUXVAL)
+#include <sys/auxv.h>
+#endif
#include "port/pg_crc32c.h"
-
-static sigjmp_buf illegal_instruction_jump;
-
-/*
- * Probe by trying to execute pg_comp_crc32c_armv8(). If the instruction
- * isn't available, we expect to get SIGILL, which we can trap.
- */
-static void
-illegal_instruction_handler(SIGNAL_ARGS)
-{
- siglongjmp(illegal_instruction_jump, 1);
-}
-
static bool
pg_crc32c_armv8_available(void)
{
- uint64 data = 42;
- int result;
-
- /*
- * Be careful not to do anything that might throw an error while we have
- * the SIGILL handler set to a nonstandard value.
- */
- pqsignal(SIGILL, illegal_instruction_handler);
- if (sigsetjmp(illegal_instruction_jump, 1) == 0)
- {
- /* Rather than hard-wiring an expected result, compare to SB8 code */
- result = (pg_comp_crc32c_armv8(0, &data, sizeof(data)) ==
- pg_comp_crc32c_sb8(0, &data, sizeof(data)));
- }
- else
- {
- /* We got the SIGILL trap */
- result = -1;
- }
- pqsignal(SIGILL, SIG_DFL);
+#if defined(HAVE_ELF_AUX_INFO)
+ unsigned long value;
-#ifndef FRONTEND
- /* We don't expect this case, so complain loudly */
- if (result == 0)
- elog(ERROR, "crc32 hardware and software results disagree");
-
- elog(DEBUG1, "using armv8 crc32 hardware = %d", (result > 0));
+ return elf_aux_info(AT_HWCAP, &value, sizeof(value)) == 0 &&
+ (value & HWCAP_CRC32) != 0;
+#elif defined(HAVE_GETAUXVAL)
+ return (getauxval(AT_HWCAP) & HWCAP_CRC32) != 0;
+#else
+ return false;
#endif
-
- return (result > 0);
}
/*
--
2.46.2
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-07 15:28 Nathan Bossart <nathandbossart@gmail.com>
parent: Thomas Munro <thomas.munro@gmail.com>
1 sibling, 1 reply; 37+ messages in thread
From: Nathan Bossart @ 2024-11-07 15:28 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Sat, Nov 02, 2024 at 06:11:33PM +1300, Thomas Munro wrote:
> On Fri, Nov 1, 2024 at 7:16 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Actually, after chewing on that second point awhile longer,
>> how about this modest proposal:
>>
>> * Drop all code for run-time determination of ARM CRC support.
>> Assume it's there unless user builds with a -march option that
>> says it definitely isn't.
>
> For CRC32, that would work (and already does via configure tests) for
> macOS (I'd forgotten about that in my earlier message, Macs never even
> compile the CRC32 "choose" code because the system compiler targets M1
> by default), recent RHEL (see below), and presumably others, but it
> would penalise Debian, FreeBSD (using the standard binary package
> repo) and others if we didn't have the runtime check due to their
> conservative choice of baseline arch (as Bastien just said about
> Debian in a crossed email). I've been wondering about what to do
> about all this for a while... There are also other features we aren't
> using yet, but should, or single instructions that we are calling
> through function pointers, preventing automatic vectorisation etc.
One small improvement in this area that I'm working on (and about to commit
for the AVX-512 stuff) is using __attribute__((target(...))) instead of
special -march options for specific files [0]. Among other things, this
simplifies the configure checks and eliminates the need to put chunks of
code in separate files. However, I found that until recently, it wasn't
possible to include arm_acle.h without the -march flag [1], which means we
likely won't be able to make the switch for the ARM CRC stuff for a few
more years.
More to the point, until the baseline moves, I think we'll end up adding
checks in header files so that we only take the function pointer overhead
when it's likely to be worth it (e.g., pg_popcount() in pg_bitutils.h). It
might be worth doing that for the CRC-32C code, too, since we are still
quite reliant on the runtime check.
[0] https://commitfest.postgresql.org/50/5305/
[1] https://postgr.es/m/ZwXsE0KG_wh6_heU%40nathan
--
nathan
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-07 15:51 Bastien Roucariès <rouca@debian.org>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Bastien Roucariès @ 2024-11-07 15:51 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
Le jeudi 7 novembre 2024, 00:01:58 UTC Thomas Munro a écrit :
> It doesn't look like I need to include <asm/hwcap.h> from Bastien's
> original message, because <sys/auxv.h> pulls in <bits/hwcap.h>
Not on arm32 unfortunatly.... and it is a HWCAP2_flags
Bastien
Attachments:
[application/pgp-signature] signature.asc (832B, ../../4188473.jn8P2z2zlG@portable-bastien/2-signature.asc)
download
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-07 23:43 Thomas Munro <thomas.munro@gmail.com>
parent: Bastien Roucariès <rouca@debian.org>
0 siblings, 1 reply; 37+ messages in thread
From: Thomas Munro @ 2024-11-07 23:43 UTC (permalink / raw)
To: Bastien Roucariès <rouca@debian.org>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
On Fri, Nov 8, 2024 at 4:51 AM Bastien Roucariès <rouca@debian.org> wrote:
> Le jeudi 7 novembre 2024, 00:01:58 UTC Thomas Munro a écrit :
> > It doesn't look like I need to include <asm/hwcap.h> from Bastien's
> > original message, because <sys/auxv.h> pulls in <bits/hwcap.h>
> Not on arm32 unfortunatly.... and it is a HWCAP2_flags
Ah, that is unfortunate. I was confused about whether we even used
this code on armv7/aarch32 builds, but I see that we do. OK, here is
a version like that.
I also noticed that the Linux docs explicitly tell you that SIGILL is
unreliable for this purpose:
https://docs.kernel.org/arch/arm64/elf_hwcaps.html
Attachments:
[application/octet-stream] v2-0001-Use-auxv-to-check-for-CRC32-instructions-on-ARM.patch (6.2K, ../../CA+hUKGKrfMUuhQNqT5WhQYfu5JhtKkfW62WbELTnLi8xbk9=Kg@mail.gmail.com/2-v2-0001-Use-auxv-to-check-for-CRC32-instructions-on-ARM.patch)
download | inline diff:
From ab53efd7767a48e0d48de985d435c5dc0d977c42 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Wed, 6 Nov 2024 22:05:13 +0000
Subject: [PATCH v2] Use auxv to check for CRC32 instructions on ARM.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously we probed for CRC32 instructions by testing if they raised
SIGILL. Some have expressed doubts about that technique, the Linux
documentation advises not to do that, and it's not exactly beautiful.
Now that more operating systems expose CPU features to userspace via the
ELF loader following the Linux conventions, let's use that instead.
This is expected to work on Linux, FreeBSD and OpenBSD.
For macOS we don't use this code at all, because its compiler already
assumes a higher -march level by default and our configure tests detect
that and use CRC32 builtins directly without probing first; therefore we
don't bother to implement the macOS way of probing.
For NetBSD, CRC32 instruction probing is lost with this commit when
using a conservative -march setting. Patches are welcome, but if NetBSD
adopts the same API as FreeBSD and OpenBSD this change should hopefully
work there too without change.
Suggested-by: Bastien Roucariès <rouca@debian.org>
Discussion: https://postgr.es/m/4496616.iHFcN1HehY%40portable-bastien
---
configure | 2 +-
configure.ac | 2 +
meson.build | 2 +
src/include/pg_config.h.in | 6 +++
src/port/pg_crc32c_armv8_choose.c | 66 +++++++++++--------------------
5 files changed, 34 insertions(+), 44 deletions(-)
diff --git a/configure b/configure
index 4b01b682b15..4d08dd59990 100755
--- a/configure
+++ b/configure
@@ -15041,7 +15041,7 @@ fi
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-for ac_func in backtrace_symbols copyfile copy_file_range getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l
+for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info getauxval getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.ac b/configure.ac
index e7f4f0fc22f..b0c3366d5e7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1717,6 +1717,8 @@ AC_CHECK_FUNCS(m4_normalize([
backtrace_symbols
copyfile
copy_file_range
+ elf_aux_info
+ getauxval
getifaddrs
getpeerucred
inet_pton
diff --git a/meson.build b/meson.build
index 5b0510cef78..b64d253fe41 100644
--- a/meson.build
+++ b/meson.build
@@ -2623,7 +2623,9 @@ func_checks = [
# when enabling asan the dlopen check doesn't notice that -ldl is actually
# required. Just checking for dlsym() ought to suffice.
['dlsym', {'dependencies': [dl_dep], 'define': false}],
+ ['elf_aux_info'],
['explicit_bzero'],
+ ['getauxval'],
['getifaddrs'],
['getopt', {'dependencies': [getopt_dep, gnugetopt_dep], 'skip': always_replace_getopt}],
['getopt_long', {'dependencies': [getopt_dep, gnugetopt_dep], 'skip': always_replace_getopt_long}],
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index cdd9a6e9355..a903c60a3a0 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -125,6 +125,9 @@
/* Define to 1 if you have the <editline/readline.h> header file. */
#undef HAVE_EDITLINE_READLINE_H
+/* Define to 1 if you have the `elf_aux_info' function. */
+#undef HAVE_ELF_AUX_INFO
+
/* Define to 1 if you have the <execinfo.h> header file. */
#undef HAVE_EXECINFO_H
@@ -154,6 +157,9 @@
*/
#undef HAVE_GCC__SYNC_INT64_CAS
+/* Define to 1 if you have the `getauxval' function. */
+#undef HAVE_GETAUXVAL
+
/* Define to 1 if you have the `getifaddrs' function. */
#undef HAVE_GETIFADDRS
diff --git a/src/port/pg_crc32c_armv8_choose.c b/src/port/pg_crc32c_armv8_choose.c
index eaf39cf3e81..147c6533e90 100644
--- a/src/port/pg_crc32c_armv8_choose.c
+++ b/src/port/pg_crc32c_armv8_choose.c
@@ -24,57 +24,37 @@
#include "postgres_fe.h"
#endif
-#include <setjmp.h>
-#include <signal.h>
+#if defined(HAVE_ELF_AUX_INFO) || defined(HAVE_GETAUXVAL)
+#include <sys/auxv.h>
+#if defined(__linux__) && !defined(__aarch64__)
+#include <asm/hwcap.h>
+#endif
+#endif
#include "port/pg_crc32c.h"
-
-static sigjmp_buf illegal_instruction_jump;
-
-/*
- * Probe by trying to execute pg_comp_crc32c_armv8(). If the instruction
- * isn't available, we expect to get SIGILL, which we can trap.
- */
-static void
-illegal_instruction_handler(SIGNAL_ARGS)
-{
- siglongjmp(illegal_instruction_jump, 1);
-}
-
static bool
pg_crc32c_armv8_available(void)
{
- uint64 data = 42;
- int result;
+#if defined(HAVE_ELF_AUX_INFO)
+ unsigned long value;
- /*
- * Be careful not to do anything that might throw an error while we have
- * the SIGILL handler set to a nonstandard value.
- */
- pqsignal(SIGILL, illegal_instruction_handler);
- if (sigsetjmp(illegal_instruction_jump, 1) == 0)
- {
- /* Rather than hard-wiring an expected result, compare to SB8 code */
- result = (pg_comp_crc32c_armv8(0, &data, sizeof(data)) ==
- pg_comp_crc32c_sb8(0, &data, sizeof(data)));
- }
- else
- {
- /* We got the SIGILL trap */
- result = -1;
- }
- pqsignal(SIGILL, SIG_DFL);
-
-#ifndef FRONTEND
- /* We don't expect this case, so complain loudly */
- if (result == 0)
- elog(ERROR, "crc32 hardware and software results disagree");
-
- elog(DEBUG1, "using armv8 crc32 hardware = %d", (result > 0));
+#ifdef __aarch64__
+ return elf_aux_info(AT_HWCAP, &value, sizeof(value)) == 0 &&
+ (value & HWCAP_CRC32) != 0;
+#else
+ return elf_aux_info(AT_HWCAP2, &value, sizeof(value)) == 0 &&
+ (value & HWCAP2_CRC32) != 0;
+#endif
+#elif defined(HAVE_GETAUXVAL)
+#ifdef __aarch64__
+ return (getauxval(AT_HWCAP) & HWCAP_CRC32) != 0;
+#else
+ return (getauxval(AT_HWCAP2) & HWCAP2_CRC32) != 0;
+#endif
+#else
+ return false;
#endif
-
- return (result > 0);
}
/*
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-08 08:56 Bastien Roucariès <rouca@debian.org>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Bastien Roucariès @ 2024-11-08 08:56 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
Le jeudi 7 novembre 2024, 23:43:45 UTC Thomas Munro a écrit :
> On Fri, Nov 8, 2024 at 4:51 AM Bastien Roucariès <rouca@debian.org> wrote:
> > Le jeudi 7 novembre 2024, 00:01:58 UTC Thomas Munro a écrit :
> > > It doesn't look like I need to include <asm/hwcap.h> from Bastien's
> > > original message, because <sys/auxv.h> pulls in <bits/hwcap.h>
>
> > Not on arm32 unfortunatly.... and it is a HWCAP2_flags
>
> Ah, that is unfortunate. I was confused about whether we even used
> this code on armv7/aarch32 builds, but I see that we do. OK, here is
> a version like that.
I will guard the include by replacing
+#if defined(__linux__) && !defined(__aarch64__)
by
+#if defined(__linux__) && !defined(__aarch64__) && !defined(HWCAP2_CRC32)
>
> I also noticed that the Linux docs explicitly tell you that SIGILL is
> unreliable for this purpose:
>
> https://docs.kernel.org/arch/arm64/elf_hwcaps.html
>
Bastien
Attachments:
[application/pgp-signature] signature.asc (832B, ../../16872859.Zp9ysIU8Tl@portable-bastien/2-signature.asc)
download
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-10 09:19 Thomas Munro <thomas.munro@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Munro @ 2024-11-10 09:19 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Fri, Nov 8, 2024 at 4:28 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
> One small improvement in this area that I'm working on (and about to commit
> for the AVX-512 stuff) is using __attribute__((target(...))) instead of
> special -march options for specific files [0].
(Catching up with that stuff). Neat.
I tried sprinkling __attribute__((target_clones("default,lse"))) in
front of some functions like LWLockAquire() just to see if this
multifunction stuff worked enough yet on ARM. FreeBSD 15/clang 18:
yes, I got LSE CAS-based lwlocks with loader-time feature detection!
Debian 12/gcc 12: no, it fails with "error: target does not support
function version dispatcher". I think maybe gcc 14 will do it (from
quick look at release notes)? I realise that's yet a different issue
than the <arm_acle.h> visibility thing you mentioned for the CRC32
builtins. Anyway, it's apparently still too soon to help on all
systems with this specific topic as you already said, so the explicit
auxv check still looks like the only reasonable way for now.
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-22 10:11 Thomas Munro <thomas.munro@gmail.com>
parent: Bastien Roucariès <rouca@debian.org>
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Munro @ 2024-11-22 10:11 UTC (permalink / raw)
To: Bastien Roucariès <rouca@debian.org>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
On Fri, Nov 8, 2024 at 9:56 PM Bastien Roucariès <rouca@debian.org> wrote:
> I will guard the include by replacing
> +#if defined(__linux__) && !defined(__aarch64__)
> by
> +#if defined(__linux__) && !defined(__aarch64__) && !defined(HWCAP2_CRC32)
Committed like that. Thanks!
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=aac831cafa
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-23 01:39 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
1 sibling, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-23 01:39 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
Thomas Munro <thomas.munro@gmail.com> writes:
> Unfortunately it looks like NetBSD doesn't put AT_HWCAP into its
> auxv[], or even expose it to user space nicely, and those libraries
> don't seem to know of another way. Hopefully NetBSD will align with
> those other systems for portability's sake. The only other way I
> could find in a quick googling session is /usr/sbin/cpuctl, root only,
> no cigar (but a solid clue that the information is floating around
> somewhere, it just depends where exactly the root-only fencing is
> happening).
I poked into this with NetBSD and found that cpuctl's "identify"
option works just fine without root. So that motivated me to dig
into its source code, and I end up with the attached. Sadly it only
works in 64-bit; there is presumably a way to detect this in 32-bit
as well, but cpuctl doesn't know it. That doesn't bother me a whole
lot though, at least not nearly as much as the 64-bit case. Anybody
running PG on 32-bit ARM shouldn't be expecting great performance.
I've checked this on NetBSD 9.2 and 10.0. I don't have hardware
that should return false (and there may not be any such 64-bit
hardware anyway...), so there's not much more I can test.
regards, tom lane
Attachments:
[text/x-diff] netbsd-arm-crc-detection.patch (1.7K, ../../707008.1732325995@sss.pgh.pa.us/2-netbsd-arm-crc-detection.patch)
download | inline diff:
diff --git a/src/port/pg_crc32c_armv8_choose.c b/src/port/pg_crc32c_armv8_choose.c
index 306500154e..f035ac9855 100644
--- a/src/port/pg_crc32c_armv8_choose.c
+++ b/src/port/pg_crc32c_armv8_choose.c
@@ -31,6 +31,11 @@
#endif
#endif
+#if defined(__NetBSD__) && defined(__aarch64__)
+#include <sys/sysctl.h>
+#include <aarch64/armreg.h>
+#endif
+
#include "port/pg_crc32c.h"
static bool
@@ -52,6 +57,42 @@ pg_crc32c_armv8_available(void)
#else
return (getauxval(AT_HWCAP2) & HWCAP2_CRC32) != 0;
#endif
+#elif defined(__NetBSD__) && defined(__aarch64__)
+ /*
+ * On NetBSD we can read AArch64 Instruction Set Attribute Register 0 via
+ * sysctl. We could probably do something similar on arm32 as well, but
+ * it's undocumented. Note we assume cpu0 is representative of all the
+ * machine's CPUs.
+ */
+ const char *path = "machdep.cpu0.cpu_id";
+ size_t len;
+#define SYSCTL_CPU_ID_MAXSIZE 64
+ uint64 sysctlbuf[SYSCTL_CPU_ID_MAXSIZE];
+ struct aarch64_sysctl_cpu_id *id =
+ (struct aarch64_sysctl_cpu_id *) sysctlbuf;
+ uint64 reg;
+ uint64 fld;
+
+ len = sizeof(sysctlbuf);
+ memset(sysctlbuf, 0, len);
+ if (sysctlbyname(path, id, &len, NULL, 0) != 0)
+ return false;
+ if (len != sizeof(struct aarch64_sysctl_cpu_id))
+ return false; /* kernel incompatibility? */
+
+#define ISAR0_CRC32_BITPOS 16
+#define ISAR0_CRC32_BITWIDTH 4
+#define WIDTHMASK(w) ((((uint64) 1) << (w)) - 1)
+
+ reg = id->ac_aa64isar0;
+ fld = (reg >> ISAR0_CRC32_BITPOS) & WIDTHMASK(ISAR0_CRC32_BITWIDTH);
+
+ /*
+ * Available documentation defines only the field values 0 (No CRC32) and
+ * 1 (CRC32B/CRC32H/CRC32W/CRC32X/CRC32CB/CRC32CH/CRC32CW/CRC32CX). Assume
+ * that any future nonzero value will be a superset of 1.
+ */
+ return (fld != 0);
#else
return false;
#endif
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-23 05:10 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 37+ messages in thread
From: Thomas Munro @ 2024-11-23 05:10 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Sat, Nov 23, 2024 at 2:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I poked into this with NetBSD and found that cpuctl's "identify"
> option works just fine without root. So that motivated me to dig
> into its source code, and I end up with the attached. Sadly it only
> works in 64-bit; there is presumably a way to detect this in 32-bit
> as well, but cpuctl doesn't know it. That doesn't bother me a whole
> lot though, at least not nearly as much as the 64-bit case. Anybody
> running PG on 32-bit ARM shouldn't be expecting great performance.
(Nerd sniped) No NetBSD here but think a 32 bit kernel should expose
sysctl machdep.id_isar, which should be an array of 6 ints, and bits
16-19 of id_isar[5] should give you the answer:
https://github.com/NetBSD/src/blob/312631d82871c81540b81abea3baa09313478bd3/sys/arch/arm/arm32/arm32...
https://github.com/NetBSD/src/blob/312631d82871c81540b81abea3baa09313478bd3/sys/arch/arm/arm32/cpu.c...
https://developer.arm.com/documentation/ddi0601/2024-09/AArch32-Registers/ID-ISAR5--Instruction-Set-...
A real 32 bit armv7 or similar like RPI2 must say no. An armv8+ like
RPI3+ running a 32 bit kernel (ie a 64 bit chip running in aarch32
mode) should say yes.
With a 64 bit kernel running a 32 bit process, I think your code
should work, except that it wouldn't be used because it's gated on
__aarch64__.
So there are combinations to consider. However, beyond satisfying
curiosity about how it works, I agree that it's all a complete waste
of time and what you have is perfectly fine. (I wonder why there
isn't a sysctl that gives you the id_aa64isar0_el1 register more
directly, instead of having to slurp it out of struct
aarch64_sysctl_cpu_id. I mean, why expose similar "isar" registers
differently for aarch32 and aarch64 kernels... but OK.)
+#define ISAR0_CRC32_BITPOS 16
+#define ISAR0_CRC32_BITWIDTH 4
+#define WIDTHMASK(w) ((((uint64) 1) << (w)) - 1)
+
+ reg = id->ac_aa64isar0;
+ fld = (reg >> ISAR0_CRC32_BITPOS) & WIDTHMASK(ISAR0_CRC32_BITWIDTH);
+
+ /*
+ * Available documentation defines only the field values 0 (No CRC32) and
+ * 1 (CRC32B/CRC32H/CRC32W/CRC32X/CRC32CB/CRC32CH/CRC32CW/CRC32CX). Assume
+ * that any future nonzero value will be a superset of 1.
+ */
+ return (fld != 0);
Since you included <aarch64/armreg.h>, and only care about non-zero,
couldn't you use the mask from the header, instead of all that
bitswizzling?
return (id->ac_aa64isar0 & ID_AA64ISAR0_EL1_CRC32) != 0;
> I've checked this on NetBSD 9.2 and 10.0. I don't have hardware
> that should return false (and there may not be any such 64-bit
> hardware anyway...), so there's not much more I can test.
I wish qemu supported a cpu that didn't have that...
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-23 05:50 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-23 05:50 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
Thomas Munro <thomas.munro@gmail.com> writes:
> (Nerd sniped) No NetBSD here but think a 32 bit kernel should expose
> sysctl machdep.id_isar, which should be an array of 6 ints, and bits
> 16-19 of id_isar[5] should give you the answer:
Huh. cpuctl's arm32 module knows of machdep.id_isar, but it does not
appear to know that that word exists:
http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/cpuctl/arch/arm.c?rev=1.6;content-type=text%2Fplain
But that developer.arm.com page seems pretty definitive. Tomorrow
I'll have a go at spinning up a 32-bit NetBSD installation and test
it. Thanks for the research!
> Since you included <aarch64/armreg.h>, and only care about non-zero,
> couldn't you use the mask from the header, instead of all that
> bitswizzling?
> return (id->ac_aa64isar0 & ID_AA64ISAR0_EL1_CRC32) != 0;
I didn't actually look into that header ;-) ... I was just basing this
on what cpuctl's code does. That version does look cleaner, at
least for aarch64 --- but given that the field is defined to be
in the same place in the relevant register for each arch, I'm
a bit inclined to keep the bit-swizzling and just have two paths
for getting the register contents. Unless maybe that header is
also installed in arm32 builds, in which case we could do it as
you show for both arches. I'll find out mañana.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-23 22:02 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-23 22:02 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
I wrote:
> But that developer.arm.com page seems pretty definitive. Tomorrow
> I'll have a go at spinning up a 32-bit NetBSD installation and test
> it. Thanks for the research!
Conclusions:
1. Unsurprisingly, a 64-bit kernel exposes machdep.cpuN.cpu_id but
not machdep.id_isar; a 32-bit kernel the reverse.
2. 32-bit userland does not provide <aarch64/armreg.h>. (So I did
not try to use the ID_AA64ISAR0_EL1_CRC32 macro: we'd have had to
hard-code the field position anyway for 32-bit.)
3. It doesn't look to me like NetBSD really supports 32-bit userland
under a 64-bit kernel. Maybe it'd kind of work, but utilities like
cpuctl seem to be built for only one of the kernel APIs.
So I end up proposing the attached. If we do somehow find ourselves
running 32-bit with a 64-bit kernel, this will report "no CRC"
because the sysctl call will fail. I don't think it's worth
sweating over that case.
BTW, I found out that by default we'll build a software-CRC-only
implementation on 32-bit ARM NetBSD, at least in the "generic"
eabihf build:
checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=... no
checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=-march=armv8-a+crc... no
checking which CRC-32C implementation to use... slicing-by-8
It turns out that -march=armv8-a+crc would work, except:
configure:17624: checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=-march=armv8-a+crc
configure:17648: ccache gcc -o conftest -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -march=armv8-a+crc -Wl,-z,now -Wl,-z,relro conftest.c -lz -ledit -lcurses -lpthread -lexecinfo -lrt -lm >&5
cc1: error: '-mfloat-abi=hard': selected processor lacks an FPU
If you force it with a compatible -mfpu switch (I used -mfpu=neon)
then you get a build with runtime CRC check, and all seems to work.
I did not try very hard to determine what the default -mfpu is here
or why -march=armv8-a+crc overrides whatever the default is. This
probably means that no end user is going to see any benefit from this
exercise, because they are not likely to try adding -mfpu by itself.
If they add "-march=armv8-a+crc -mfpu=whatever" to CFLAGS manually
then configure will deem that a runtime check is not required, so this
code still doesn't get exercised.
tl;dr: a lot of time spent on a probably-useless exercise. Still,
now that we've got the code we may as well commit it. The configure
issue was there all along.
regards, tom lane
Attachments:
[text/x-diff] v2-netbsd-arm-crc-detection.patch (2.0K, ../../865773.1732399354@sss.pgh.pa.us/2-v2-netbsd-arm-crc-detection.patch)
download | inline diff:
diff --git a/src/port/pg_crc32c_armv8_choose.c b/src/port/pg_crc32c_armv8_choose.c
index 306500154e..11e27978fd 100644
--- a/src/port/pg_crc32c_armv8_choose.c
+++ b/src/port/pg_crc32c_armv8_choose.c
@@ -31,6 +31,13 @@
#endif
#endif
+#if defined(__NetBSD__)
+#include <sys/sysctl.h>
+#if defined(__aarch64__)
+#include <aarch64/armreg.h>
+#endif
+#endif
+
#include "port/pg_crc32c.h"
static bool
@@ -52,6 +59,47 @@ pg_crc32c_armv8_available(void)
#else
return (getauxval(AT_HWCAP2) & HWCAP2_CRC32) != 0;
#endif
+#elif defined(__NetBSD__)
+ /*
+ * On NetBSD we can read the Instruction Set Attribute Registers via
+ * sysctl. For doubtless-historical reasons the sysctl interface is
+ * completely different on 64-bit than 32-bit, but the underlying
+ * registers contain the same fields.
+ */
+#define ISAR0_CRC32_BITPOS 16
+#define ISAR0_CRC32_BITWIDTH 4
+#define WIDTHMASK(w) ((1 << (w)) - 1)
+#define SYSCTL_CPU_ID_MAXSIZE 64
+
+ size_t len;
+ uint64 sysctlbuf[SYSCTL_CPU_ID_MAXSIZE];
+#if defined(__aarch64__)
+ /* We assume cpu0 is representative of all the machine's CPUs. */
+ const char *path = "machdep.cpu0.cpu_id";
+ size_t expected_len = sizeof(struct aarch64_sysctl_cpu_id);
+#define ISAR0 ((struct aarch64_sysctl_cpu_id *) sysctlbuf)->ac_aa64isar0
+#else
+ const char *path = "machdep.id_isar";
+ size_t expected_len = 6 * sizeof(int);
+#define ISAR0 ((int *) sysctlbuf)[5]
+#endif
+ uint64 fld;
+
+ len = sizeof(sysctlbuf);
+ memset(sysctlbuf, 0, len);
+ if (sysctlbyname(path, sysctlbuf, &len, NULL, 0) != 0)
+ return false; /* perhaps kernel is 64-bit and we aren't? */
+ if (len != expected_len)
+ return false; /* kernel API change? */
+
+ fld = (ISAR0 >> ISAR0_CRC32_BITPOS) & WIDTHMASK(ISAR0_CRC32_BITWIDTH);
+
+ /*
+ * Current documentation defines only the field values 0 (No CRC32) and 1
+ * (CRC32B/CRC32H/CRC32W/CRC32X/CRC32CB/CRC32CH/CRC32CW/CRC32CX). Assume
+ * that any future nonzero value will be a superset of 1.
+ */
+ return (fld != 0);
#else
return false;
#endif
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-23 23:50 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 37+ messages in thread
From: Thomas Munro @ 2024-11-23 23:50 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Sun, Nov 24, 2024 at 11:02 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> cc1: error: '-mfloat-abi=hard': selected processor lacks an FPU
Interesting. Happens also on Linux starting with GCC 11, apparently.
Here's one with some clues:
https://github.com/checkpoint-restore/criu/issues/1653#issuecomment-986034428
We have a few Linux armv7 animals in the farm, but using GCC 10 and
older so not seeing it yet.
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-24 00:08 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-24 00:08 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
Thomas Munro <thomas.munro@gmail.com> writes:
> On Sun, Nov 24, 2024 at 11:02 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> cc1: error: '-mfloat-abi=hard': selected processor lacks an FPU
> Interesting. Happens also on Linux starting with GCC 11, apparently.
> Here's one with some clues:
> https://github.com/checkpoint-restore/criu/issues/1653#issuecomment-986034428
Oh, very interesting! Maybe we need to try -march=armv8-a+crc+fp
(or some spelling like that) if -march=armv8-a+crc doesn't work?
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-24 19:07 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 37+ messages in thread
From: Thomas Munro @ 2024-11-24 19:07 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Sun, Nov 24, 2024 at 1:09 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Oh, very interesting! Maybe we need to try -march=armv8-a+crc+fp
> (or some spelling like that) if -march=armv8-a+crc doesn't work?
Hmm, they were talking about armv7-a, not armv8-a, but it seems
related... does it work?
The GCC docs give only +nofp, not +fp as an option for 32 bit armv8-a
(that is, under "ARM options", as opposed to "aarch64 options"). But
I guess GCC is pretty configurable and NetBSD targets some weird
hardware... as for why it could be like that (if not a mistake), it
looks like it might be technically possible for armv8 (including -a)
to lack FP in aarch32 state (unlike aarch64 state), but I went looking
for real existing 32-bit only armv8 chips lacking FP and found only
stuff like Cortex M23, and they're using armv8-m (embedded profile,
lots of other stuff missing or optional). The weakest 32 bit-only
armv8-a chip I could find in a quick search was the Cortex A32 but it
looks like it has FP. I don't know enough about any of this stuff to
guess what's going on here.
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-24 20:16 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-24 20:16 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
Thomas Munro <thomas.munro@gmail.com> writes:
> The GCC docs give only +nofp, not +fp as an option for 32 bit armv8-a
> (that is, under "ARM options", as opposed to "aarch64 options"). But
> I guess GCC is pretty configurable and NetBSD targets some weird
> hardware... as for why it could be like that (if not a mistake), it
> looks like it might be technically possible for armv8 (including -a)
> to lack FP in aarch32 state (unlike aarch64 state), but I went looking
> for real existing 32-bit only armv8 chips lacking FP and found only
> stuff like Cortex M23, and they're using armv8-m (embedded profile,
> lots of other stuff missing or optional). The weakest 32 bit-only
> armv8-a chip I could find in a quick search was the Cortex A32 but it
> looks like it has FP. I don't know enough about any of this stuff to
> guess what's going on here.
I'm confused too, because the gcc documentation is quite definite
that -march=armv8-a assumes FP support by default. It seems like
somebody broke that for no very good reason and didn't bother to
update the docs either. Maybe it's actually a bug?
Mainline Linux distros like Fedora and Debian seem to have dropped
32-bit ARM altogether, but there's still a build available from
https://www.raspberrypi.com/software/operating-systems/
that says it is based on Debian bookworm. I installed that on
my RPi4 and found that it has
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/arm-linux-gnueabihf/12/lto-wrapper
Target: arm-linux-gnueabihf
Configured with: ... --with-arch=armv6+fp --with-float=hard ...
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.0 (Raspbian 12.2.0-14+rpi1)
So, just like NetBSD's "generic" ARM32 build, they are firmly in
the hard-float camp. There may be builds out there that will
still work on soft-float ARM, but they've got to be scarce ...
and you'd have to wonder why anybody would be trying to run PG
on such hardware anyway.
Anyway, experimentation shows that this gcc version gives the same
"selected processor lacks an FPU" failure with "-march=armv8-a+crc".
It will not take "-march=armv8-a+crc+fp", but it will take
"-march=armv8-a+crc -mfpu=vfpv3"; the same holds for NetBSD 10.0's
gcc 10.5.0. Possibly other -mfpu settings will work (I recall having
had success yesterday with -mfpu=neon on NetBSD), but I think this is
a pretty generic setting: per the gcc docs it corresponds to typical
armv7 hardware.
I've checked the attached patch on both the raspberrypi system and
NetBSD 10.0.
regards, tom lane
Attachments:
[text/x-diff] improve-arm-crc-configure-check.patch (4.5K, ../../1074317.1732479406@sss.pgh.pa.us/2-improve-arm-crc-configure-check.patch)
download | inline diff:
diff --git a/configure b/configure
index 28719ed30c..d61a7e59dd 100755
--- a/configure
+++ b/configure
@@ -17578,8 +17578,10 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
# Check for ARMv8 CRC Extension intrinsics to do CRC calculations.
#
# First check if __crc32c* intrinsics can be used with the default compiler
-# flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_CRC is set if the extra flag is required.
+# flags. If not, check if adding -march=armv8-a+crc flag helps. On systems
+# using hard-float ABI, it may be that "-march=armv8-a+crc" will be rejected
+# but "-march=armv8-a+crc -mfpu=vfpv3" will work, so next try that.
+# CFLAGS_CRC is set if extra flag(s) are required.
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=" >&5
$as_echo_n "checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=... " >&6; }
if ${pgac_cv_armv8_crc32c_intrinsics_+:} false; then :
@@ -17661,6 +17663,48 @@ if test x"$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" = x"yes"; then
pgac_armv8_crc32c_intrinsics=yes
fi
+ if test x"$pgac_armv8_crc32c_intrinsics" != x"yes"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=-march=armv8-a+crc -mfpu=vfpv3" >&5
+$as_echo_n "checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=-march=armv8-a+crc -mfpu=vfpv3... " >&6; }
+if ${pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc__mfpu_vfpv3+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+CFLAGS="$pgac_save_CFLAGS -march=armv8-a+crc -mfpu=vfpv3"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+#include <arm_acle.h>
+int
+main ()
+{
+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;
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+ pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc__mfpu_vfpv3=yes
+else
+ pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc__mfpu_vfpv3=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext conftest.$ac_ext
+CFLAGS="$pgac_save_CFLAGS"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc__mfpu_vfpv3" >&5
+$as_echo "$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc__mfpu_vfpv3" >&6; }
+if test x"$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc__mfpu_vfpv3" = x"yes"; then
+ CFLAGS_CRC="-march=armv8-a+crc -mfpu=vfpv3"
+ pgac_armv8_crc32c_intrinsics=yes
+fi
+
+ fi
fi
# Check for LoongArch CRC intrinsics to do CRC calculations.
diff --git a/configure.ac b/configure.ac
index 533f4ab78a..7bed68c7de 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2093,11 +2093,16 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [
# Check for ARMv8 CRC Extension intrinsics to do CRC calculations.
#
# First check if __crc32c* intrinsics can be used with the default compiler
-# flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_CRC is set if the extra flag is required.
+# flags. If not, check if adding -march=armv8-a+crc flag helps. On systems
+# using hard-float ABI, it may be that "-march=armv8-a+crc" will be rejected
+# but "-march=armv8-a+crc -mfpu=vfpv3" will work, so next try that.
+# CFLAGS_CRC is set if extra flag(s) are required.
PGAC_ARMV8_CRC32C_INTRINSICS([])
if test x"$pgac_armv8_crc32c_intrinsics" != x"yes"; then
PGAC_ARMV8_CRC32C_INTRINSICS([-march=armv8-a+crc])
+ if test x"$pgac_armv8_crc32c_intrinsics" != x"yes"; then
+ PGAC_ARMV8_CRC32C_INTRINSICS([-march=armv8-a+crc -mfpu=vfpv3])
+ fi
fi
# Check for LoongArch CRC intrinsics to do CRC calculations.
diff --git a/meson.build b/meson.build
index b64d253fe4..4bf730e40a 100644
--- a/meson.build
+++ b/meson.build
@@ -2294,6 +2294,13 @@ int main(void)
cdata.set('USE_ARMV8_CRC32C', false)
cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
have_optimized_crc = true
+ elif cc.links(prog, name: '__crc32cb, __crc32ch, __crc32cw, and __crc32cd with -march=armv8-a+crc -mfpu=vfpv3',
+ args: test_c_args + ['-march=armv8-a+crc', '-mfpu=vfpv3'])
+ # Use ARM CRC Extension, with runtime check
+ cflags_crc += ['-march=armv8-a+crc', '-mfpu=vfpv3']
+ cdata.set('USE_ARMV8_CRC32C', false)
+ cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
+ have_optimized_crc = true
endif
elif host_cpu == 'loongarch64'
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-24 22:50 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 37+ messages in thread
From: Thomas Munro @ 2024-11-24 22:50 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Mon, Nov 25, 2024 at 9:16 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Anyway, experimentation shows that this gcc version gives the same
> "selected processor lacks an FPU" failure with "-march=armv8-a+crc".
> It will not take "-march=armv8-a+crc+fp", but it will take
> "-march=armv8-a+crc -mfpu=vfpv3"; the same holds for NetBSD 10.0's
> gcc 10.5.0. Possibly other -mfpu settings will work (I recall having
> had success yesterday with -mfpu=neon on NetBSD), but I think this is
> a pretty generic setting: per the gcc docs it corresponds to typical
> armv7 hardware.
LGTM.
It might be slightly more logical to say vfpv4 if that also works,
since armv8 floats are based on vfpf4[1], but it hardly matters given
we're not actually using it... [1] is also where you can read that
software implementation is still technically permitted in armv8
aarch32 state (but not aarch64), even though A profile chips without
it are apparently like unicorns. Anyway, I can't find any evidence
that GCC knows that[2], or changed that deliberately any time
recently, or is out of sync with its own docs, or has local patches in
NetBSD's copy, so yeah, maybe it really is a bug. There was some
recent-ish churn affecting -mfpu and -march interactions[3], maybe
relevant, or not, it beats me.
[1] https://developer.arm.com/documentation/dui0801/l/Overview-of-the-Armv8-Architecture/Floating-point-...
[2] https://github.com/gcc-mirror/gcc/blob/master/gcc/config/arm/arm-cpus.in#L583
[3] https://sourceware.org/bugzilla/show_bug.cgi?id=28078
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-24 23:34 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 2 replies; 37+ messages in thread
From: Tom Lane @ 2024-11-24 23:34 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
Thomas Munro <thomas.munro@gmail.com> writes:
> On Mon, Nov 25, 2024 at 9:16 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Anyway, experimentation shows that this gcc version gives the same
>> "selected processor lacks an FPU" failure with "-march=armv8-a+crc".
>> It will not take "-march=armv8-a+crc+fp", but it will take
>> "-march=armv8-a+crc -mfpu=vfpv3"; the same holds for NetBSD 10.0's
>> gcc 10.5.0. Possibly other -mfpu settings will work (I recall having
>> had success yesterday with -mfpu=neon on NetBSD), but I think this is
>> a pretty generic setting: per the gcc docs it corresponds to typical
>> armv7 hardware.
> LGTM.
> It might be slightly more logical to say vfpv4 if that also works,
> since armv8 floats are based on vfpf4[1], but it hardly matters given
> we're not actually using it...
Yeah, with no FP operations in the files that get compiled with these
flags, it probably doesn't matter. But gcc 10.5.0 seems happy with
-mfpu=vfpv4, so I'll use that.
> [1] is also where you can read that
> software implementation is still technically permitted in armv8
> aarch32 state (but not aarch64), even though A profile chips without
> it are apparently like unicorns. Anyway, I can't find any evidence
> that GCC knows that[2], or changed that deliberately any time
> recently, or is out of sync with its own docs, or has local patches in
> NetBSD's copy, so yeah, maybe it really is a bug.
It's very confusing, especially given this:
$ gcc -march=armv8-a+crc+fp testarm32.c
gcc: error: 'armv8-a' does not support feature 'fp'
gcc: note: valid feature names are: crc simd crypto nocrypto nofp sb predres; did you mean 'nofp'?
If they think that base armv8-a doesn't include FP, why is there a
+nofp option? But whether it's a bug or somehow intentional, there's
an awful lot of copies of gcc with this behavior, so we need to cope.
I wonder whether "armv8-a" includes FP on aarch64 but not arm32, and
we only did the original testing of f044d71e3 on aarch64. If so,
it might've been like this all along and we didn't notice.
> There was some
> recent-ish churn affecting -mfpu and -march interactions[3], maybe
> relevant, or not, it beats me.
That looks like incorrect assembly code being emitted. Our
problem here is much further upstream, ie what compiler flags
are accepted.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-25 00:40 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 37+ messages in thread
From: Tom Lane @ 2024-11-25 00:40 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
I wrote:
> It's very confusing, especially given this:
> $ gcc -march=armv8-a+crc+fp testarm32.c
> gcc: error: 'armv8-a' does not support feature 'fp'
> gcc: note: valid feature names are: crc simd crypto nocrypto nofp sb predres; did you mean 'nofp'?
> If they think that base armv8-a doesn't include FP, why is there a
> +nofp option?
Bug filed at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117766
We'll see what they say, but it seems like we need a workaround
in any case.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-25 00:47 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 37+ messages in thread
From: Thomas Munro @ 2024-11-25 00:47 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
On Mon, Nov 25, 2024 at 12:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wonder whether "armv8-a" includes FP on aarch64 but not arm32, and
> we only did the original testing of f044d71e3 on aarch64. If so,
> it might've been like this all along and we didn't notice.
It does work on the "armv7" target BF animals, though. I assume they
don't get any updates as they're all old/defunct distros (one of my
favourite features of our boneyard^Wbuildfarm), but one of them is GCC
"10.2.1 20210110", which is what made me suspicious of that -mfpu
stuff, beyond my comprehension without a lot more coffee, that got
backpatched in late 2021.
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-25 01:27 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-25 01:27 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
Thomas Munro <thomas.munro@gmail.com> writes:
> It does work on the "armv7" target BF animals, though. I assume they
> don't get any updates as they're all old/defunct distros (one of my
> favourite features of our boneyard^Wbuildfarm), but one of them is GCC
> "10.2.1 20210110", which is what made me suspicious of that -mfpu
> stuff, beyond my comprehension without a lot more coffee, that got
> backpatched in late 2021.
Ah, you're looking at mereswine, which is hard-float according to
checking build system type... armv7l-unknown-linux-gnueabihf
checking host system type... armv7l-unknown-linux-gnueabihf
I also confirmed that our existing code works on Fedora 30 armv7l
(also hard-float) with gcc 9.3.1. That's the only other ARM32
image I have at hand :-(
So yeah, it would seem they broke it between 10.2.1 and 10.5.0.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-25 15:41 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-25 15:41 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
I wrote:
> I also confirmed that our existing code works on Fedora 30 armv7l
> (also hard-float) with gcc 9.3.1. That's the only other ARM32
> image I have at hand :-(
> So yeah, it would seem they broke it between 10.2.1 and 10.5.0.
After discussion with the gcc folk (see bug report linked upthread)
it emerges that there was indeed a change somewhere in that time
frame. It used to be that if the platform defaults were
"-march=A -mfpu=X" then writing "-march=B" on the command line
would still leave you with "-mfpu=X", possibly a nonsensical
combination. So now they ignore any build-time default, with
the effect that writing just "-march=armv8-a+crc" selects soft
float, which fails on platforms that require hard-float ABI.
(I didn't ask whether this was the same change prompted by the
other bug report you found, but perhaps it was.)
Also, there's no +fp option for "-march=armv8-a" because the
ARMv8 specs require chips to implement both or neither of FP
and SIMD. So the idiomatic thing for us to write is not a
separate -mfpu spec but "-march=armv8-a+crc+simd". I'll
adjust my patch to do it like that and push later today.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-25 17:53 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-25 17:53 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
I wrote:
> Also, there's no +fp option for "-march=armv8-a" because the
> ARMv8 specs require chips to implement both or neither of FP
> and SIMD. So the idiomatic thing for us to write is not a
> separate -mfpu spec but "-march=armv8-a+crc+simd". I'll
> adjust my patch to do it like that and push later today.
And done. In the event I decided to try the +simd option first,
reasoning that that's correct on the majority of current ARM systems.
I'm interested to see what the older buildfarm ARM animals pick.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-25 22:40 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 37+ messages in thread
From: Tom Lane @ 2024-11-25 22:40 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
I wrote:
> And done. In the event I decided to try the +simd option first,
> reasoning that that's correct on the majority of current ARM systems.
> I'm interested to see what the older buildfarm ARM animals pick.
Early returns are that they're all picking the +simd option, and
seem to be just as happy with that as they were without it.
I wonder if we could just drop the "without" probe altogether.
It'd save some configure cycles for other platforms (since they'll
fruitlessly run through all of these tests).
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Detection of hadware feature => please do not use signal
@ 2024-11-27 19:57 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 37+ messages in thread
From: Tom Lane @ 2024-11-27 19:57 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Bastien Roucariès <rouca@debian.org>; pgsql-bugs@lists.postgresql.org
I wrote:
> Early returns are that they're all picking the +simd option, and
> seem to be just as happy with that as they were without it.
> I wonder if we could just drop the "without" probe altogether.
> It'd save some configure cycles for other platforms (since they'll
> fruitlessly run through all of these tests).
Now that we have a nearly-complete census of the buildfarm,
the end result is that they are all happy with the +simd
spelling ... except for one. "gull" rejects it:
clang: error: unsupported argument 'armv8-a+crc+simd' to option '-march='
But without +simd it's okay, so removing the other spelling
would cause a performance regression on that machine.
I find this a bit surprising, because gull is using a reasonably
recent compiler (clang version 17.0.4). We have much older
clang versions that accept +simd --- but they are all aarch64.
gull seems to be our only representative of clang on arm32.
So this seems like a gcc vs clang difference that we'll have
to put up with.
Anyway, I think this thread is now closed: nothing more to do
unless someone thinks of a platform we need more bespoke code
for.
regards, tom lane
^ permalink raw reply [nested|flat] 37+ messages in thread
end of thread, other threads:[~2024-11-27 19:57 UTC | newest]
Thread overview: 37+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-10-31 15:41 Detection of hadware feature => please do not use signal Bastien Roucariès <rouca@debian.org>
2024-10-31 17:29 ` Heikki Linnakangas <hlinnaka@iki.fi>
2024-10-31 17:42 ` Bastien Roucariès <rouca@debian.org>
2024-10-31 20:16 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-10-31 17:48 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-10-31 18:01 ` Heikki Linnakangas <hlinnaka@iki.fi>
2024-10-31 18:25 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-10-31 20:12 ` Bastien Roucariès <rouca@debian.org>
2024-11-01 04:28 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-01 04:44 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-01 06:16 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-01 10:20 ` Bastien Roucariès <rouca@debian.org>
2024-11-02 05:11 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-07 00:01 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-07 15:51 ` Bastien Roucariès <rouca@debian.org>
2024-11-07 23:43 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-08 08:56 ` Bastien Roucariès <rouca@debian.org>
2024-11-22 10:11 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-07 15:28 ` Nathan Bossart <nathandbossart@gmail.com>
2024-11-10 09:19 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-23 01:39 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-23 05:10 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-23 05:50 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-23 22:02 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-23 23:50 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-24 00:08 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-24 19:07 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-24 20:16 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-24 22:50 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-24 23:34 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-25 00:40 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-25 00:47 ` Thomas Munro <thomas.munro@gmail.com>
2024-11-25 01:27 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-25 15:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-25 17:53 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-25 22:40 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-27 19:57 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox