Re: [PATCH] crc: New optimised slice-by-8 implementation

[Bruno Haible]

Hi Sam,

> Sorry for the multiple bumps, but this patch offers a 77% reduction in
> compute time for CRC an a 35% reduction in gzip time,

Thanks for following through!

> if this patch is not compliant then can some please let me know

I would suggest three things:

1) Add into comments references to the research papers that are used in
   or are useful to understand the code. Like the "Intel paper" that you
   showed us in the beginning.

2) Generate the tables at build time.
   Rationale: big tables with hex numbers are frowned upon, because they
   are not "source code" (in the sense of the GPL: "the preferred form of
   the work for making modifications to it").

   Step 1: Move the tables into a file lib/crc-sliceby8.h, that you
           #include from lib/crc.c. Add a copyright and license notice,
           like for every file.
   Step 2: Extend your program lib/crc-generate-table.c so that it generates
           the entire lib/crc-sliceby8.h, i.e. including the copyright and
           license notice. For this, you can copy & adapt the code from
           lib/gen-uni-tables.c, functions output_library_license and
           output_predicate.
   Step 3: Write a Makefile.am rule, in modules/crc, that runs the generator.
           Take a look at modules/uninorm/composition (there it's gperf as
           generator; it doesn't make a big difference).

3) Use GNU coding style:

     printf(    -->   printf (

     int main(int argc, char* argv[])
                -->
     int
     main (int argc, char* argv[])

     Indent by 2 spaces, not mixed 2 / 4 spaces.


Thanks!

Bruno