RE: [PATCH] add slice by 8 algorithm to crc32.c

["Bob Pearson" <rpearson@systemfabricworks.com>]

Happy to consider this. I have been asking the list for comments about the
idea of dropping the BITS=2 and BITS=4 algorithms altogether which would
make the table size just 256. So far no one has claimed that they actually
care about those algorithms except as 'examples'. The 'Sarwate' algorithm
(which I added as an 8 bit version) is faster and only adds 2x4KB of table.

If no one really uses the smaller but much slower versions I don't see a
reason to keep them.

> -----Original Message-----
> From: George Spelvin [mailto:linux@horizon.com]
> Sent: Monday, August 08, 2011 7:55 AM
> To: joakim.tjernlund@transmode.se; linux@horizon.com
> Cc: akpm@linux-foundation.org; fzago@systemfabricworks.com; linux-
> kernel@vger.kernel.org; rpearson@systemfabricworks.com
> Subject: Re: [PATCH] add slice by 8 algorithm to crc32.c
> 
> > -#define LE_TABLE_SIZE (1 << CRC_LE_BITS)
> > -#define BE_TABLE_SIZE (1 << CRC_BE_BITS)
> > +#if CRC_LE_BITS > 8
> > +# define LE_TABLE_SIZE 256
> > +#else
> > +# define LE_TABLE_SIZE (1 << CRC_LE_BITS)
> > +#endif
> > +#if CRC_BE_BITS > 8
> > +# define BE_TABLE_SIZE 256
> > +#else
> > +# define BE_TABLE_SIZE (1 << CRC_BE_BITS)
> > +#endif
> >
> > -static uint32_t crc32table_le[4][LE_TABLE_SIZE];
> > -static uint32_t crc32table_be[4][BE_TABLE_SIZE];
> > +#define LE_TABLE_ROWS ((CRC_LE_BITS - 1)/8 + 1)
> > +#define BE_TABLE_ROWS ((CRC_BE_BITS - 1)/8 + 1)
> > +
> > +static uint32_t crc32table_le[LE_TABLE_ROWS][LE_TABLE_SIZE];
> > +static uint32_t crc32table_be[BE_TABLE_ROWS][BE_TABLE_SIZE];
> 
> Minor cleanup suggestion: The two different ways of computing
> xE_TABLE_SIZE and xE_TABLE_ROWS is a bit confusing.
> 
> May I recommend choosing one of the following:
> 
> #if CRC_LE_BITS > 8
> # define LE_TABLE_ROWS (CRC_LE_BITS/8)
> # define LE_TABLE_SIZE 256
> #else
> # define LE_TABLE_ROWS 1
> # define LE_TABLE_SIZE (1 << CRC_LE_BITS)
> #endif
> 
> or
> 
> #define LE_TABLE_ROWS ((CRC_LE_BITS - 1)/8 + 1)
> #define LE_TABLE_SIZE (1 << ((CRC_LE_BITS - 1)%8 + 1))
> 
> Either one makes the relationship between the two clearer.