Horton

Thread

Commits

  1. Update configure probes for CFLAGS needed for ARM CRC instructions.

  2. Support runtime CRC feature probing on NetBSD/ARM using sysctl().

  3. Use auxv to check for CRC32 instructions on ARM.

  4. Improve our method for probing the availability of ARM CRC instructions.

  1. Detection of hadware feature => please do not use signal

    Bastien Roucariès <rouca@debian.org> — 2024-10-31T15:41:24Z

    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

  2. Re: Detection of hadware feature => please do not use signal

    Heikki Linnakangas <hlinnaka@iki.fi> — 2024-10-31T17:29:45Z

    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%40mail.gmail.com

-- 
Heikki Linnakangas
Neon (https://neon.tech)

  3. Re: Detection of hadware feature => please do not use signal

    Bastien Roucariès <rouca@debian.org> — 2024-10-31T17:42:09Z

    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%40mail.gmail.com
> 
>

  4. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-10-31T17:48:48Z

    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

  5. Re: Detection of hadware feature => please do not use signal

    Heikki Linnakangas <hlinnaka@iki.fi> — 2024-10-31T18:01:41Z

    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)

  6. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-10-31T18:25:34Z

    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

  7. Re: Detection of hadware feature => please do not use signal

    Bastien Roucariès <rouca@debian.org> — 2024-10-31T20:12:12Z

    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#L311

# 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
>

  8. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-10-31T20:16:49Z

    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

  9. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-01T04:28:53Z

    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%40mail.gmail.com
[2] https://www.postgresql.org/message-id/flat/20240612193746.rjeiip4hcamjedgo%40awork3.anarazel.de#ab383730597411817c69516ec6c1a65c

  10. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-01T04:44:35Z

    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

  11. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-01T06:16:32Z

    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

  12. Re: Detection of hadware feature => please do not use signal

    Bastien Roucariès <rouca@debian.org> — 2024-11-01T10:20:03Z

    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
>

  13. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-02T05:11:33Z

    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%40mail.gmail.com

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...

  14. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-07T00:01:58Z

    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.

  15. Re: Detection of hadware feature => please do not use signal

    Nathan Bossart <nathandbossart@gmail.com> — 2024-11-07T15:28:44Z

    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

  16. Re: Detection of hadware feature => please do not use signal

    Bastien Roucariès <rouca@debian.org> — 2024-11-07T15:51:46Z

    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

  17. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-07T23:43:45Z

    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

  18. Re: Detection of hadware feature => please do not use signal

    Bastien Roucariès <rouca@debian.org> — 2024-11-08T08:56:45Z

    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

  19. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-10T09:19:54Z

    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.

  20. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-22T10:11:42Z

    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

  21. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-23T01:39:55Z

    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

  22. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-23T05:10:17Z

    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_machdep.c#L142
https://github.com/NetBSD/src/blob/312631d82871c81540b81abea3baa09313478bd3/sys/arch/arm/arm32/cpu.c#L869
https://developer.arm.com/documentation/ddi0601/2024-09/AArch32-Registers/ID-ISAR5--Instruction-Set-Attribute-Register-5?lang=en

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...

  23. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-23T05:50:46Z

    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

  24. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-23T22:02:34Z

    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

  25. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-23T23:50:09Z

    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.

  26. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-24T00:08:55Z

    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

  27. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-24T19:07:26Z

    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.

  28. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-24T20:16:46Z

    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

  29. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-24T22:50:53Z

    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-hardware?lang=en
[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

  30. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-24T23:34:28Z

    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

  31. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-25T00:40:29Z

    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

  32. Re: Detection of hadware feature => please do not use signal

    Thomas Munro <thomas.munro@gmail.com> — 2024-11-25T00:47:06Z

    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.

  33. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-25T01:27:13Z

    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

  34. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-25T15:41:37Z

    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

  35. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-25T17:53:58Z

    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

  36. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-25T22:40:10Z

    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

  37. Re: Detection of hadware feature => please do not use signal

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-11-27T19:57:03Z

    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