Re: [V2] Adding new CRC32C implementation for IBM S390X - Mailing list pgsql-hackers

From John Naylor
Subject Re: [V2] Adding new CRC32C implementation for IBM S390X
Date
Msg-id CANWCAZb367jqTgg+s+LCbV78DbsBqWDW22+9t-=rUPC65TPUQg@mail.gmail.com
Whole thread Raw
In response to Re: [V2] Adding new CRC32C implementation for IBM S390X  (Eduard Stefes <eddy@linux.ibm.com>)
List pgsql-hackers
On Fri, Jul 11, 2025 at 7:01 PM Eduard Stefes <eddy@linux.ibm.com> wrote:
>
> On Wed, 2025-07-09 at 13:53 +0700, John Naylor wrote:
> > v3 still has direct-call and runtime-check paths. Let's keep only
> > USE_S390X_CRC32C_WITH_RUNTIME_CHECK and discard the direct call
> > configure checks. Once that's done I'll take a closer look and test
> > as
> > well. The rest should be small details.
>
> done :) here is V5 with only runtime-check.

Great, thanks! I took this (v4, for the archives) for a spin on one of
the LinuxONE instances in the buildfarm. Since godbolt.org doesn't
have clang for s390x, I tested on a machine with clang 18. With that,
I found that the configure check successfully compiles and links
broken code. :-( This is different from gcc 14 which "harmlessly"
failed to compile (until 14.3 fixed that). So, it seems for the clang
versions that are broken we actually do need to guard within the test
programs after all. Eduard, which compiler versions have you tested
the patch on?

I found the patch passes tests with gcc 13.3 on this machine, then
looked at the configuration outputs.

configure:

```
checking for __attribute__((vector_size(16))), vec_gfmsum_accum_128
with CFLAGS=-fzvector -march=z13... no
checking for __attribute__((vector_size(16))), vec_gfmsum_accum_128
with CFLAGS=-mzarch -march=z13... yes
checking which CRC-32C implementation to use... S390X Vector
instructions for CRC with runtime check
checking for vectorized CRC-32C... none
```

This looks strange. I think we want "CRC-32C implementation" to report
"slicing-by-8" and "vectorized CRC-32C" to report "S390X Vector
instructions".

+# Check for S390X Vector intrinsics to do CRC calculations.
+#
+# First check for the host cpu
+if test x"$host_cpu" = x"s390x" && test x"$ac_cv_func_getauxval" = x"yes"; then

I believe ac_cv_func_getauxval is a leftover from the previous patch.

meson:

```
Checking for function "getauxval" : YES
Checking if "s390x_vector_vx+march=z13" : links: YES
```

This looks fine except for the "getauxval", which is due to the duplicate check:

+elif host_cpu == 's390x' and cc.has_function('getauxval')

This check already happened at the beginning of the file within
"func_checks" and defined the corresponding HAVE_ symbol, which v4
seems to use appropriately in the runtime check.

--
John Naylor
Amazon Web Services



pgsql-hackers by date:

Previous
From: vignesh C
Date:
Subject: Re: Logical Replication of sequences
Next
From: "yexiu-glory"
Date:
Subject: PostgreSQL 16 bug feedback