On 14/06/2023 05:14, Paul Eggert wrote:
Thanks for the bug report. I installed the attached patch into coreutils
on Savannah. It builds on your idea with several other changes:
* There's a similar issue with cksum.c and pclmul.
* configure.ac can be simplified, since it seems there's no point
compiling these instructions if __builtin_cpu_supports doesn't work.
* This lets us simplify the source code a bit more.
Please let me know if the attached patch works for you.
__builtin_cpu_supports() looks to have sufficient support in
Arch + compiler versions for our needs, so that's good.
Paul you removed the "avx" check from cksum.c. Was that intended?
PS. Does the attached cksum.c / pclmul change fix any user-visible
misbehavior? If so, what should we put into the NEWS file?
We have an illegal instruction issue with cksum under Xen DomU
which may be related, as discussed at: https://bugs.debian.org/1037264
Axel does the attached patch change anything for you?
thanks,
Pádraig
diff --git a/src/cksum.c b/src/cksum.c
index 85afab0ac..881d90413 100644
--- a/src/cksum.c
+++ b/src/cksum.c
@@ -160,29 +160,16 @@ static bool
pclmul_supported (void)
{
# if USE_PCLMUL_CRC32
- unsigned int eax = 0;
- unsigned int ebx = 0;
- unsigned int ecx = 0;
- unsigned int edx = 0;
-
- if (! __get_cpuid (1, &eax, &ebx, &ecx, &edx))
- {
- if (cksum_debug)
- error (0, 0, "%s", _("failed to get cpuid"));
- return false;
- }
-
- if (! (ecx & bit_PCLMUL) || ! (ecx & bit_AVX))
- {
- if (cksum_debug)
- error (0, 0, "%s", _("pclmul support not detected"));
- return false;
- }
+ bool pclmul_enabled = 0 < __builtin_cpu_supports ("pclmul")
+ && 0 < __builtin_cpu_supports ("avx");
if (cksum_debug)
- error (0, 0, "%s", _("using pclmul hardware support"));
+ error (0, 0, "%s",
+ (pclmul_enabled
+ ? _("using pclmul hardware support")
+ : _("pclmul support not detected")));
- return true;
+ return pclmul_enabled;
# else
if (cksum_debug)
error (0, 0, "%s", _("using generic hardware support"));
