Re: [PATCH] cksum: Use pclmul hardware instruction for CRC32 calculation

[Pádraig Brady]

On 14/03/2021 13:36, Kristoffer Brånemyr wrote:
> Hi,
>
> Ok, good to know it's progressing!
>
> I think that it would be best not to compile the main cksum.c with the -mavx .. 
> flags. With those flags I think the compiler is free to insert AVX instructions 
> in the main cksum.o file, which would fail on a CPU not supporting it. Think 
> for instance of a package maintainer of a linux distributions, the compiler 
> that person used would support -mavx so the flag is enabled by autoconf, but 
> another person using the resulting binary might have a CPU not supporting AVX. 
> That's what I tried to avoid by only having the cksum_pclmul.c enabling the 
> compiler flags, plus runtime detection.

Fair point. Ok I will keep the -mavx flags separate with:

diff --git a/src/local.mk b/src/local.mk
index d4a00a1a4..4ff9b22c3 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -361,8 +361,10 @@ src_coreutils_SOURCES = src/coreutils.c
 src_cksum_SOURCES = src/cksum.c src/cksum.h
-src_cksum_CFLAGS = $(AM_CFLAGS)
 if USE_PCLMUL_CRC32
-src_cksum_SOURCES += src/cksum_pclmul.c
-src_cksum_CFLAGS += -mavx -mpclmul
+noinst_LIBRARIES += src/libcksum_pclmul.a
+src_libcksum_pclmul_a_SOURCES = src/cksum_pclmul.c src/cksum.h
+src_cksum_LDADD += src/libcksum_pclmul.a
+src_libcksum_pclmul_a_CFLAGS = -mavx -mpclmul $(AM_CFLAGS)
 endif
 src_cp_SOURCES = src/cp.c $(copy_sources) $(selinux_sources)
 src_dir_SOURCES = src/ls.c src/ls-dir.c

In addition to support how we process ..._LDADD variables in coreutils,
when building the single binary mode,
we'll also add in an extra level of indirection as follows,
to ensure libcksum_pclmul.a is only referenced if built.

-src_cksum_LDADD += src/libcksum_pclmul.a
+cksum_pclmul_ldadd = src/libcksum_pclmul.a
+src_cksum_LDADD += $(cksum_pclmul_ldadd)

There is another gotcha with using a separate library,
in how dependencies are managed when building in single binary mode.
A full build proceeds fine, but to ensure all components are _built_
for partial builds, we'll need to propagate ..._DEPENDENCIES
to the single binary mode libs. I'll do that first in the attached.

thanks,
Pádraig

single-DEPENDENCIES.patch
Description: Text Data