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 <br...@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:
