Received-SPF: pass (google.com: domain of linux-kernel+bounces-27618-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: David Laight <David.Laight@ACULAB.COM>
To: 'Andrew Jones' <ajones@ventanamicro.com>, Xiao Wang
	<xiao.w.wang@intel.com>
CC: "paul.walmsley@sifive.com" <paul.walmsley@sifive.com>,
	"palmer@dabbelt.com" <palmer@dabbelt.com>, "aou@eecs.berkeley.edu"
	<aou@eecs.berkeley.edu>, "conor.dooley@microchip.com"
	<conor.dooley@microchip.com>, "heiko@sntech.de" <heiko@sntech.de>,
	"haicheng.li@intel.com" <haicheng.li@intel.com>,
	"linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] riscv: Optimize crc32 with Zbc extension
Thread-Topic: [PATCH] riscv: Optimize crc32 with Zbc extension
Thread-Index: AQHaSI1C2Wp5kEQM1k+UoNSZlBGjlrDcqukw
Date: Tue, 16 Jan 2024 17:15:56 +0000
Message-ID: <1153d2dd82cf43adb6062627d8d89b27@AcuMS.aculab.com>
References: <20240105080830.3738117-1-xiao.w.wang@intel.com>
 <20240116-9f09b002afc2337ab5e41e3f@orel>
In-Reply-To: <20240116-9f09b002afc2337ab5e41e3f@orel>
Accept-Language: en-GB, en-US
Precedence: bulk
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

..
> > +static inline u32 __pure crc32_le_generic(u32 crc, unsigned char const=
 *p,
> > +#if (BITS_PER_LONG =3D=3D 64)
> > +=09=09=09=09=09  size_t len, u32 poly, u64 poly_qt,
> > +#else
> > +=09=09=09=09=09  size_t len, u32 poly, u32 poly_qt,
> > +#endif
>=20
> How about creating a new type for poly_qt, defined as u64 for xlen=3D64
> and u32 for xlen=3D32 to avoid the #ifdef?

unsigned long ?

..
> > +=09for (int i =3D 0; i < len; i++) {
> > +#if (BITS_PER_LONG =3D=3D 64)
> > +=09=09s =3D (unsigned long)crc << 32;
> > +=09=09s ^=3D __cpu_to_be64(*p_ul++);
> > +#else
> > +=09=09s =3D crc ^ __cpu_to_be32(*p_ul++);
> > +#endif
>=20
> Could write the above without #ifdef with

Haven't I seen a bpf patch that rather implies that byteswap
is likely to be truly horrid?

I've not tried to parse the crc code (although I do understand
how it should work). But I'm surprised you need a byteswap.

After all, the crc is basically a long division of the buffer
by the crc constant.

The CRC I've done recently is the hdlc crc-16.
My nios version (also mips-like) has:

static __inline__ uint32_t
crc_step(uint32_t crc, uint32_t byte_val)
{
#if defined(crc_step_ci)
    return crc_step_ci(byte_val, crc);
#else
    uint32_t t =3D crc ^ (byte_val & 0xff);
    t =3D (t ^ t << 4) & 0xff;
    return crc >> 8 ^ t << 8 ^ t << 3 ^ t >> 4;
#endif
}

I normally use a custom instruction for the logic - one clock.
But the C code is only a couple of clocks slower that the best
table lookup version.
On anything pipelined and multi-issue the C code is likely to
be faster than a lookup table!
I don't know if any of the 32bit crc can be reduced the same way.

=09David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1=
PT, UK
Registration No: 1397386 (Wales)