On Wed, Sep 19, 2018 at 7:07 AM Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> > I reviewed 'pgbench-prp-func/pgbench-prp-func-4.patch'. [...] I thinks
> > this patch is fine.
modular_multiply() is an interesting device. I will leave this to
committers with a stronger mathematical background than me, but I have
a small comment in passing:
+#ifdef HAVE__BUILTIN_POPCOUNTLL
+ return __builtin_popcountll(n);
+#else /* use clever no branching bitwise operator version */
I think it is not enough for the compiler to have
__builtin_popcountll(). The CPU that this is eventually executed on
must actually have the POPCNT instruction[1] (or equivalent on ARM,
POWER etc), or the program will die with SIGILL. There are CPUs in
circulation produced in this decade that don't have it. I have
previously considered something like this[2], but realised you would
therefore need a runtime check. There are a couple of ways to do
that: see commit a7a73875 for one example, also
__builtin_cpu_supports(), and direct CPU ID bit checks (some
platforms). There is also the GCC "multiversion" system that takes
care of that for you but that worked only for C++ last time I checked.
[1] https://en.wikipedia.org/wiki/SSE4#POPCNT_and_LZCNT
[2] https://www.postgresql.org/message-id/CAEepm%3D3g1_fjJGp38QGv%2B38BC2HHVkzUq6s69nk3mWLgPHqC3A%40mail.gmail.com
--
Thomas Munro
http://www.enterprisedb.com