Re: crc-x86_64: Fix compilation error with clang

[Bruno Haible]

Hi Collin,

> Would __attribute__ ((target ("..."))) not work here?

Indeed, clang supports __attribute__ ((__target__ ("pclmul,avx")))
at least since version 3.9 (from 2016). Thanks for reminding us of
this alternative!

Which of the two approaches is better? I think this boils down
to the question: Which of the two approaches generalizes better
to other compilers?

Looking at possible MSVC support: MSVC does not need specific
options (although /Oi can be used) [1], and just needs a different
#include [2]. What that simple change, crc-x86_64-pclmul.c compiles
fine. The only remaining problem is that that compiler does not have
the __builtin_cpu_supports primitive and instead recommends to
"Use the __cpuid intrinsic to determine instruction-set support at run time."
[2]. But I don't think it is the job of an application programmer to write
code like [3].

So, all in all, it seems that the use of __attribute__ and #pragmas
is easier to maintain, in the long run, than compiler options.

I'm therefore applying the simplification below.

[1] https://learn.microsoft.com/en-us/cpp/intrinsics/compiler-intrinsics
[2] https://learn.microsoft.com/en-us/cpp/intrinsics/x64-amd64-intrinsics-list
[3] https://learn.microsoft.com/en-us/cpp/intrinsics/cpuid-cpuidex


2024-12-21  Bruno Haible  <bruno@clisp.org>

        crc-x86_64: Fix compilation error with clang in a simpler way.
        Suggested by Collin Funk.
        * modules/crc-x86_64 (Makefile.am): Revert last change.
        * lib/crc-x86_64-pclmul.c: Normalize includes.
        (crc32_update_no_xor_pclmul): Use __attribute__ ((__target (...))).
        * m4/crc-x86_64.m4 (gl_CRC_X86_64_PCLMUL): Use
        __attribute__ ((__target (...))) here as well. Don't use modified
        CFLAGS.

diff --git a/lib/crc-x86_64-pclmul.c b/lib/crc-x86_64-pclmul.c
index 91f361d2ce..170ebfcef0 100644
--- a/lib/crc-x86_64-pclmul.c
+++ b/lib/crc-x86_64-pclmul.c
@@ -18,12 +18,17 @@
 
 #include <config.h>
 
+/* Specification.  */
 #include "crc-x86_64.h"
-#include "x86intrin.h"
-#include "crc.h"
 
 #include <string.h>
+#include <x86intrin.h>
+
+#include "crc.h"
 
+#if defined __GNUC__ || defined __clang__
+__attribute__ ((__target__ ("pclmul,avx")))
+#endif
 uint32_t
 crc32_update_no_xor_pclmul (uint32_t crc, const void *buf, size_t len)
 {
diff --git a/m4/crc-x86_64.m4 b/m4/crc-x86_64.m4
index fa649a5b82..470e6333ff 100644
--- a/m4/crc-x86_64.m4
+++ b/m4/crc-x86_64.m4
@@ -1,5 +1,5 @@
 # crc-x86_64.m4
-# serial 2
+# serial 3
 dnl Copyright (C) 2024 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -8,14 +8,15 @@
 
 AC_DEFUN([gl_CRC_X86_64_PCLMUL],
 [
-  ac_save_CFLAGS=$CFLAGS
-  CFLAGS="-mavx -mpclmul $CFLAGS"
   AC_CACHE_CHECK([if pclmul intrinsic exists], [gl_cv_crc_pclmul], [
     AC_LINK_IFELSE(
       [AC_LANG_SOURCE(
         [[
           #include <x86intrin.h>
 
+          #if defined __GNUC__ || defined __clang__
+          __attribute__ ((__target__ ("pclmul,avx")))
+          #endif
           int
           main (void)
           {
@@ -37,5 +38,4 @@ AC_DEFUN([gl_CRC_X86_64_PCLMUL]
   fi
   AM_CONDITIONAL([GL_CRC_X86_64_PCLMUL],
                  [test $gl_cv_crc_pclmul = yes])
-  CFLAGS=$ac_save_CFLAGS
 ])
diff --git a/modules/crc-x86_64 b/modules/crc-x86_64
index 15876991ab..2e4b5bf744 100644
--- a/modules/crc-x86_64
+++ b/modules/crc-x86_64
@@ -15,17 +15,7 @@ AC_REQUIRE([gl_CRC_X86_64_PCLMUL])
 
 Makefile.am:
 if GL_CRC_X86_64_PCLMUL
-# We need a separate library, in order to compile crc-x86_64-pclmul.c with
-# particular CFLAGS.
-# (Recall that '#pragma GCC target (...)' works only with gcc, not with clang.
-# And the alternative approach of target-specific CFLAGS in 'make' syntax
-# <https://www.gnu.org/software/make/manual/html_node/Target_002dspecific.html>
-# is not portable: it does not work with OpenBSD 'make'.)
-noinst_@LT@LIBRARIES += libpclmul.@la@
-libpclmul_@la@_SOURCES = crc-x86_64-pclmul.c
-libpclmul_@la@_CFLAGS = $(AM_CFLAGS) -mavx -mpclmul
-lib_LIBADD += libpclmul_@la@-crc-x86_64-pclmul.@lo@
-lib_DEPENDENCIES += libpclmul.@la@
+lib_SOURCES += crc-x86_64-pclmul.c
 endif
 
 Include: