public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: "Bob Pearson" <rpearson@systemfabricworks.com>
Cc: "'frank zago'" <fzago@systemfabricworks.com>,
	<linux-kernel@vger.kernel.org>,
	"'Roland Dreier'" <roland@kernel.org>
Subject: Re: [PATCH] add slice by 8 algorithm to crc32.c
Date: Mon, 1 Aug 2011 12:39:14 -0700	[thread overview]
Message-ID: <20110801123914.5a643eb3.akpm@linux-foundation.org> (raw)
In-Reply-To: <034001cc4d91$7f232c70$7d698550$@systemfabricworks.com>

On Thu, 28 Jul 2011 20:47:36 -0500
"Bob Pearson" <rpearson@systemfabricworks.com> 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.


  reply	other threads:[~2011-08-01 19:39 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-20 22:19 [PATCH] add slice by 8 algorithm to crc32.c frank zago
2011-07-28 22:16 ` Andrew Morton
2011-07-29  1:47   ` Bob Pearson
2011-08-01 19:39     ` Andrew Morton [this message]
     [not found] <OF4AE0115F.3AA5397E-ONC12578DF.002EC6DF-C12578DF.003348E5@transmode.se>
2011-08-02 21:14 ` Bob Pearson
2011-08-02 21:19   ` Bob Pearson
2011-08-04 11:54   ` Joakim Tjernlund
2011-08-04 18:53     ` Bob Pearson
2011-08-05  9:22       ` Joakim Tjernlund
2011-08-05 15:51         ` Bob Pearson
2011-08-08  7:11           ` Joakim Tjernlund
2011-08-05 17:27         ` Bob Pearson
2011-08-08  7:15           ` Joakim Tjernlund
     [not found]       ` <OF14136E0E.3F2388EF-ONC12578E3.00301969-C12578E3.00338524@LocalDomain>
2011-08-05 13:34         ` Joakim Tjernlund
  -- strict thread matches above, loose matches on Subject: below --
2011-08-08  9:28 George Spelvin
2011-08-08 10:31 ` Joakim Tjernlund
2011-08-08 10:52   ` George Spelvin
2011-08-08 11:11     ` Joakim Tjernlund
2011-08-08 17:04       ` Bob Pearson
     [not found]     ` <OFEA1BD2B2.B2A7F07F-ONC12578E6.003D368C-C12578E6.003D7468@LocalDomain>
2011-08-08 11:24       ` Joakim Tjernlund
2011-08-08 11:42     ` Joakim Tjernlund
2011-08-08 12:54       ` George Spelvin
2011-08-08 17:01         ` Bob Pearson
2011-08-08 20:45           ` George Spelvin
2011-08-08 22:21             ` Bob Pearson
2011-08-08 16:54     ` Bob Pearson
2011-08-08 16:50 ` Bob Pearson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110801123914.5a643eb3.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=fzago@systemfabricworks.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roland@kernel.org \
    --cc=rpearson@systemfabricworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox