Messages in this thread | | | Date | Wed, 9 Nov 2011 16:15:39 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] crc32: Optimize inner loop. |
| |
On Thu, 3 Nov 2011 10:28:25 +0100 Joakim Tjernlund <Joakim.Tjernlund@transmode.se> wrote:
> taking a pointer reference to each row in the crc table matrix, > one can reduce the inner loop with a few insn's > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
As we have other developers who have previously shown an interest in the crc32 code it is a good idea to cc them on future patches to get a bit more review and perhaps testing input.
This patch seems fairly harmless. Some runtime testing results would have been appropriate.
Also, something like "reduce a few insns" is extremely easy to substantiate, so why not do it? With my compiler, the patch reduces the crc32.o text by 22 bytes.
> As the slice by 8 CRC32 optimizations seems stalled, I figured > I send in this generic one.
I meant to ask about that. The discussion was fairly long and complex and I'm now unsure whether the patch I have is a still up-to-date, whether it is a good one, whether it has additional testing results, etc.
So probably the best thing to do now is for me to drop it. Please resend when convenient. Be sure to cc all previous reviewers and also update the changelog to cover any important issues which were brought up in the review. This is to show what the issues were, how they were addressed, etc.
Thanks.
> index 4855995..b06d1e7 100644 > --- a/lib/crc32.c > +++ b/lib/crc32.c > @@ -51,20 +51,21 @@ static inline u32 > crc32_body(u32 crc, unsigned char const *buf, size_t len, const u32 (*tab)[256]) > { > # ifdef __LITTLE_ENDIAN > -# define DO_CRC(x) crc = tab[0][(crc ^ (x)) & 255] ^ (crc >> 8) > -# define DO_CRC4 crc = tab[3][(crc) & 255] ^ \ > - tab[2][(crc >> 8) & 255] ^ \ > - tab[1][(crc >> 16) & 255] ^ \ > - tab[0][(crc >> 24) & 255] > +# define DO_CRC(x) crc = t0[(crc ^ (x)) & 255] ^ (crc >> 8) > +# define DO_CRC4 crc = t3[(crc) & 255] ^ \ > + t2[(crc >> 8) & 255] ^ \ > + t1[(crc >> 16) & 255] ^ \ > + t0[(crc >> 24) & 255] > # else > -# define DO_CRC(x) crc = tab[0][((crc >> 24) ^ (x)) & 255] ^ (crc << 8) > -# define DO_CRC4 crc = tab[0][(crc) & 255] ^ \ > - tab[1][(crc >> 8) & 255] ^ \ > - tab[2][(crc >> 16) & 255] ^ \ > - tab[3][(crc >> 24) & 255] > +# define DO_CRC(x) crc = t0[((crc >> 24) ^ (x)) & 255] ^ (crc << 8) > +# define DO_CRC4 crc = t0[(crc) & 255] ^ \ > + t1[(crc >> 8) & 255] ^ \ > + t2[(crc >> 16) & 255] ^ \ > + t3[(crc >> 24) & 255] > # endif > const u32 *b; > size_t rem_len; > + const u32 *t0=tab[0], *t1=tab[1], *t2=tab[2], *t3=tab[3]; > > /* Align it */ > if (unlikely((long)buf & 3 && len)) {
| |