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
>From 5b8c7dd03de1f15d33f5598c328ea754026b139d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Sun, 14 Mar 2021 22:43:42 +0000
Subject: [PATCH] maint: propagate DEPENDENCIES to libs in single binary mode

build-aux/gen-single-binary.sh (override_single): A new function
to refactor the existing mappings for dir, vdir, and arch.
This function now also sets the DEPENDENCIES variable so that these
dependencies can be maintained later in the script, where
we now propagate the automake generated $(src_$cmd_DEPENDENCIES)
to our equivalent src_libsinglebin_$cmd_a_DEPENDENCIES.
This will ensure that any required libs are built,
which we require in a following change to cksum that
builds part of it as a separate library.
---
 build-aux/gen-single-binary.sh | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/build-aux/gen-single-binary.sh b/build-aux/gen-single-binary.sh
index 4e07cfdaa..657061535 100755
--- a/build-aux/gen-single-binary.sh
+++ b/build-aux/gen-single-binary.sh
@@ -58,19 +58,19 @@ done < $LOCAL_MK
 me=`echo "$0" | sed 's,.*/,,'`
 echo "## Automatically generated by $me.  DO NOT EDIT BY HAND!"
 
-# Override the sources for dir and vdir. We use a smaller version of dir and
-# vdir that relies on the ls main.
-src_dir_SOURCES="src/coreutils-dir.c"
-src_dir_LDADD="$src_dir_LDADD src/libsinglebin_ls.a"
-echo src_libsinglebin_dir_a_DEPENDENCIES = src/libsinglebin_ls.a
-src_vdir_SOURCES="src/coreutils-vdir.c"
-src_vdir_LDADD="$src_vdir_LDADD src/libsinglebin_ls.a"
-echo src_libsinglebin_vdir_a_DEPENDENCIES = src/libsinglebin_ls.a
-
-# Override the sources for arch likewise, using the main from uname.
-src_arch_SOURCES="src/coreutils-arch.c"
-src_arch_LDADD="$src_arch_LDADD src/libsinglebin_uname.a"
-echo src_libsinglebin_arch_a_DEPENDENCIES = src/libsinglebin_uname.a
+# Override the sources for some tools, to use smaller variants
+override_single() {
+  from="$1"; to="$2";
+
+  eval "src_${from}_SOURCES='src/coreutils-${from}.c'"
+  eval "src_from_LDADD=\$src_${from}_LDADD"
+  eval "src_${from}_LDADD='$src_from_LDADD src/libsinglebin_${to}.a'"
+  eval "src_libsinglebin_${from}_a_DEPENDENCIES='src/libsinglebin_${to}.a'"
+  echo "src_libsinglebin_${from}_a_DEPENDENCIES = src/libsinglebin_${to}.a"
+}
+override_single dir ls
+override_single vdir ls
+override_single arch uname
 
 for cmd in $ALL_PROGRAMS; do
   echo "# Command $cmd"
@@ -88,6 +88,13 @@ for cmd in $ALL_PROGRAMS; do
     echo "${base}_ldadd = $value"
   fi
 
+  # DEPENDENCIES
+  var=src_libsinglebin_${cmd}_a_DEPENDENCIES
+  eval "value=\$$var"
+  if [ "x$value" = "x" ]; then
+    echo "$var = \$(src_${cmd}_DEPENDENCIES)"
+  fi
+
   # CFLAGS
   # Hack any other program defining a main() replacing its main by
   # single_binary_main_$PROGRAM_NAME.
-- 
2.26.2

Reply via email to