Re: [PATCH v3 2/6] scripts/gen-crc-consts: add gen-crc-consts.py

[David Laight]

On Thu, 6 Feb 2025 12:08:43 -0800
Eric Biggers <ebiggers@xxxxxxxxxx> wrote:

> On Thu, Feb 06, 2025 at 07:31:17PM +0000, David Laight wrote:
> > On Wed,  5 Feb 2025 23:39:44 -0800
> > Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> >   
> > > From: Eric Biggers <ebiggers@xxxxxxxxxx>
> > > 
> > > Add a Python script that generates constants for computing the given CRC
> > > variant(s) using x86's pclmulqdq or vpclmulqdq instructions.
> > > 
> > > This is specifically tuned for x86's crc-pclmul-template.S.  However,
> > > other architectures with a 64x64 => 128-bit carryless multiplication
> > > instruction should be able to use the generated constants too.  (Some
> > > tweaks may be warranted based on the exact instructions available on
> > > each arch, so the script may grow an arch argument in the future.)
> > > 
> > > The script also supports generating the tables needed for table-based
> > > CRC computation.  Thus, it can also be used to reproduce the tables like
> > > t10_dif_crc_table[] and crc16_table[] that are currently hardcoded in
> > > the source with no generation script explicitly documented.
> > > 
> > > Python is used rather than C since it enables implementing the CRC math
> > > in the simplest way possible, using arbitrary precision integers.  The
> > > outputs of this script are intended to be checked into the repo, so
> > > Python will continue to not be required to build the kernel, and the
> > > script has been optimized for simplicity rather than performance.  
> > 
> > It might be better to output #defines that just contain array
> > initialisers rather than the definition of the actual array itself.
> > 
> > Then any code that wants the values can include the header and
> > just use the constant data it wants to initialise its own array.
> > 
> > 	David  
> 
> The pclmul constants use structs, not arrays.  Maybe you are asking for the
> script to only generate the struct initializers?

I'd not read the python that closely.

> This suggestion seems a bit more complicated than just having everything in one place.

It'll be in several places anyway since the python file is only going
to generate the lookup tables.

> It would allow
> putting the struct definitions in the CRC-variant-specific files while keeping
> the struct initializers all in one file, so __maybe_unused would no longer need
> to be used on the definitions.  But the actual result would be the same, just
> achieved in what seems like a slightly more difficult way.

It would leave the variable declarations in the file that used them - making
it easier to see what they are.
It also gives the option of minor changes in the variable name/attributes
which might be useful at some point (or some architecture).

I've got some similar tables for a normal byte-lookup crc16 (hdlc).
And for doing the hdlc bit-stuffing and flag/abort detection on
a byte-by-byte basis
The whole lot is 11k - quite a lot of memory inside an fpga!
I started with the 'header' containing the initialised data, but
later changed it to just #define for the initialiser - worked better
that way.

	David