From: Tom Lane <tgl@sss.pgh.pa.us>
To: John Naylor <john.naylor@enterprisedb.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: use ARM intrinsics in pg_lfind32() where available
Date: Sat, 27 Aug 2022 17:18:34 -0400
Message-ID: <2494641.1661635114@sss.pgh.pa.us> (raw)
In-Reply-To: <CAFBsxsF=K1Vo4fZELJMCSF8GGbcrC_FnkMt3sQcBZt=MExmjPA@mail.gmail.com>
References: <20220819222814.GA401294@nathanxps13>
<CAFBsxsEvzdUZe3WqojHDTT28HOopR4eibARdH2eCkdBEnM__qQ@mail.gmail.com>
<20220822211547.GA1126462@nathanxps13>
<CAFBsxsEN5nW3uRh=jrs-QexDrC1btu0ZfriD3FFfb=3J6tAngg@mail.gmail.com>
<20220824180111.GB1302810@nathanxps13>
<CAFBsxsE-mCC4Dxb6wmgEZTynyKtER0Y0ha_Rw_rLMY_u2j2oqg@mail.gmail.com>
<20220825045729.GA1458024@nathanxps13>
<CAFBsxsHa9QmLk33c5C-12Uuh35Ofv+qzbT+auGrqC3fQCorqBg@mail.gmail.com>
<20220826045115.GA1638993@nathanxps13>
<20220826061347.GA1777731@nathanxps13>
<20220826182403.GA1917683@nathanxps13>
<CAFBsxsF=K1Vo4fZELJMCSF8GGbcrC_FnkMt3sQcBZt=MExmjPA@mail.gmail.com>
I spent a bit more time researching the portability implications of
this patch. I think that we should check __ARM_NEON before #including
<arm_neon.h>; there is authoritative documentation out there telling
you to, eg [1], and I can see no upside at all to not checking.
We cannot check *only* __ARM_NEON, though. I found it to get defined
by clang 8.0.0 in my Fedora 30 32-bit image, although that does not
provide all the instructions we want (I see "undefined function"
complaints for vmaxvq_u8 etc if I try to make it use the patch).
Looking into that installation's <arm_neon.h>, those functions are
defined conditionally if "__ARM_FP & 2", which is kind of interesting
--- per [1], that bit indicates support for 16-bit floating point,
which seems a mite unrelated.
It appears from the info at [2] that there are at least some 32-bit
ARM platforms that set that bit, implying (if the clang authors are
well informed) that they have the instructions we want. But we
could not realistically make 32-bit builds that try to use those
instructions without a run-time test; such a build would fail for
too many people. I doubt that a run-time test is worth the trouble,
so I concur with the idea of selecting NEON on aarch64 only and hoping
to thereby avoid a runtime test.
In short, I think the critical part of 0002 needs to look more like
this:
+#elif defined(__aarch64__) && defined(__ARM_NEON)
+/*
+ * We use the Neon instructions if the compiler provides access to them
+ * (as indicated by __ARM_NEON) and we are on aarch64. While Neon support is
+ * technically optional for aarch64, it appears that all available 64-bit
+ * hardware does have it. Neon exists in some 32-bit hardware too, but
+ * we could not realistically use it there without a run-time check,
+ * which seems not worth the trouble for now.
+ */
+#include <arm_neon.h>
+#define USE_NEON
...
Coding like this appears to work on both my Apple M1 and my Raspberry
Pi, with several different OSes checked on the latter.
regards, tom lane
[1] https://developer.arm.com/documentation/101754/0618/armclang-Reference/Other-Compiler-specific-Featu...
[2] http://micro-os-plus.github.io/develop/predefined-macros/
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, john.naylor@enterprisedb.com, nathandbossart@gmail.com, andres@anarazel.de
Subject: Re: use ARM intrinsics in pg_lfind32() where available
In-Reply-To: <2494641.1661635114@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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