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

[Andrew Morton]

On Thu, 28 Jul 2011 20:47:36 -0500
"Bob Pearson" <rpearson@xxxxxxxxxxxxxxxxxxxxx> wrote:

> > 
> > > +
> > > +	init_bytes = (uintptr_t)p32 - (uintptr_t)p8;
> > 
> > Please take a look at the types of init_bytes and end_bytes.
> > ptrdiff_t, size_t and uint would all eb more appropriate than `int'.
> 
> Is there a performance difference on 32 bit machines from using a long to
> hold something that is in the range [0,7]? Which of these would you
> recommend?

I'm not aware of 64-bit being any faster than 32-bit on any machine. 
But I'm not aware of much.

Making this a signed quantity was inappropriate.  unsigned int, perhaps.

IMO, unsigned should have been the default in C - negative numbers are
relatively rare.  Whatever.

> > > +	p32 = (u32 *)(((uintptr_t)p8 + 3) & ~3);
> > 
> > ALIGN()?
> 
> Sure.
> 
> > 
> > > +	init_bytes = (uintptr_t)p32 - (uintptr_t)p8;
> > > +	if (init_bytes > len)
> > > +		init_bytes = len;
> > 
> > max()?
> 
> Maybe. First line is the main thought and the if handles the exception where
> the unrolled loop doesn't have a middle. Happy to oblige though. How about
> an unlikely in the if?

Don't care much ;) It's just that the reader looks at those two lines,
absorbs them then says "ah, it's taking the minimum".  If it had just
used "min()" then review and reading are more efficient, and reliable.  Plus min()
has typechecking features which can catch slipups.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/