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

[Pádraig Brady]

On 13/03/2021 16:13, Pádraig Brady wrote:

FYI testing on an older i3-2310M system
shows the bottleneck is not near I/O (cat is much faster).
A 500MiB file improves from 1.40s to 0.67s on the i3-2310M.

$ time src/cksum file.in
  3404199294 524288000 file.in
  real  0m0.672s
  user  0m0.584s
  sys   0m0.084s

I'm also considering applying the attached
to add a --debug option (present on a few other coreutils),
which will diagnose the implementation used
(since it's build time and run time variable).

cheers,
Pádraig

>From f43b70f294dd3f4ac2795c58c3b9b719756ccbf5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 13 Mar 2021 18:10:12 +0000
Subject: [PATCH] cksum: add --debug to diagnose which implementation used

* src/cksum.c: (main): Use getopt_long to parse options,
and handle the new --debug option.
(pclmul_supported): Diagnose various failures and attempts.
---
 src/cksum.c | 55 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/src/cksum.c b/src/cksum.c
index f776f1957..67362e09b 100644
--- a/src/cksum.c
+++ b/src/cksum.c
@@ -39,6 +39,7 @@
 
 #define AUTHORS proper_name ("Q. Frank Xia")
 
+#include <getopt.h>
 #include <stdio.h>
 #include <sys/types.h>
 #include <stdint.h>
@@ -141,7 +142,6 @@ main (void)
 
 #else /* !CRCTAB */
 
-# include "long-options.h"
 # include "die.h"
 # include "error.h"
 
@@ -153,6 +153,21 @@ main (void)
 /* Number of bytes to read at once.  */
 # define BUFLEN (1 << 16)
 
+static bool debug;
+
+enum
+{
+  DEBUG_PROGRAM_OPTION = CHAR_MAX + 1,
+};
+
+static struct option const longopts[] =
+{
+  {"debug", no_argument, NULL, DEBUG_PROGRAM_OPTION},
+  {GETOPT_HELP_OPTION_DECL},
+  {GETOPT_VERSION_OPTION_DECL},
+  {NULL, 0, NULL, 0},
+};
+
 /* Nonzero if any of the files read were the standard input. */
 static bool have_read_stdin;
 
@@ -173,13 +188,21 @@ pclmul_supported (void)
   unsigned int edx = 0;
 
   if (! __get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return false;
+    {
+      if (debug)
+        error (0, 0, "%s", _("failed to get cpuid"));
+      return false;
+    }
 
-  if (! (ecx & bit_PCLMUL))
-    return false;
+  if (! (ecx & bit_PCLMUL) || ! (ecx & bit_AVX))
+    {
+      if (debug)
+        error (0, 0, "%s", _("pclmul support not detected"));
+      return false;
+    }
 
-  if (! (ecx & bit_AVX))
-    return false;
+  if (debug)
+    error (0, 0, "%s", _("using pclmul hardware support"));
 
   return true;
 }
@@ -334,7 +357,7 @@ Print CRC checksum and byte counts of each FILE.\n\
 int
 main (int argc, char **argv)
 {
-  int i;
+  int i, c;
   bool ok;
 
   initialize_main (&argc, &argv);
@@ -349,8 +372,22 @@ main (int argc, char **argv)
      so that processes running in parallel do not intersperse their output.  */
   setvbuf (stdout, NULL, _IOLBF, 0);
 
-  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE, Version,
-                                   true, usage, AUTHORS, (char const *) NULL);
+  while ((c = getopt_long (argc, argv, "", longopts, NULL)) != -1)
+    {
+      switch (c)
+        {
+        case DEBUG_PROGRAM_OPTION:
+          debug = true;
+          break;
+
+        case_GETOPT_HELP_CHAR;
+
+        case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
+
+        default:
+          usage (EXIT_FAILURE);
+        }
+    }
 
   have_read_stdin = false;
 
-- 
2.26.2