On Mon, Mar 14, 2022 at 5:33 PM Joern Rennecke <joern.renne...@embecosm.com> wrote: > > Most microprocessors have efficient ways to perform CRC operations, be > that with lookup tables, rotates, or even special instructions. > However, because we lack a representation for CRC in the compiler, we > can't do proper instruction selection. With this patch I seek out to > rectify this, > I've avoided using a mode name for the built-in functions because that > would tie the semantics to the size of the addressable unit. We > generally use abbreviations like s/l/ll for type names, which is all > right when the type can be widened without changing semantics. For > the data input, however, we also have to consider the shift count that > is tied to it. That is why I used a number to designate the width of > the data input and shift. > > For machine support, I made a start with 8 and 16 bit little-endian > CRC for RISCV using a > lookup table. I am sure once we have the basic infrastructure in the > tree, we'll get more > contributions of suitable named patterns for various ports.
A few points. There are at least 9 different polynomials for the CRC-8 in common use today. For CRC-32 there are 5 different polynomials used. You don't have a patch to invoke.texi adding the descriptions of the builtins. How is your polynom 3rd argument described? Is it similar to how it is done on the wiki for the CRC? Does it make sense to have to list the most common polynomials in the documentation? Also I am sorry but micro-optimizing coremarks is just wrong. Maybe it is better to pick the CRC32 that is inside zip instead for a testcase and benchmarking against? Or even the CRC32C for iSCSI/ext4. I see you also don't optimize the case where you have three other variants of polynomials that are reversed, reciprocal and reversed reciocal. Also a huge problem, you don't check to make sure the third argument to the crc builtin function is constant in the rsicv backend. Plus since you expose the crc builtins as a non target specific builtin, I assume there should be a libcall right and therefore either you need to have a generic expansion for it or a function added to libgcc? Also why not expand generically to the table or a loop based on a target hook instead of in the backend? This way a target can choose without even doing much. Thanks, Andrew Pinski > > bootstrapped on x86_64-pc-linux-gnu .
