Re: [PATCH 1/4] LOG2: Implement a general integer log2 facility in the kernel [try #4]

[David Howells <dhowells@redhat.com>]

Christoph Lameter <clameter@sgi.com> wrote:

> On Mon, 9 Oct 2006, David Howells wrote:
> 
> > On FRV, for example, I don't want to wrap fls() because the code for ilog2()
> > can be shorter and simpler.
> > 
> > If I did fls() as a wrapper around ilog2() then it would have to involve a
> > conditional jump because the compiler can't alter the inline asm of
> > ilog2() to turn the SCAN instruction into CSCAN (which is a conditionally
> > executed version of SCAN).
>
> Hmmm.. Why not? If you can define fls on a per arch basis then that should 
> be possible? You can tell the compiler to produce the correct version of 
> the scan instructions. We do that frequently on IA64.

Ummm...  How?  As far as I know there's no way to do that.  Actually, it's
harder than just changing SCAN into CSCAN: you can't tell the compiler about
condition code type parameters to inline asm, so you have to embed all the
conditional bits in the inline asm if you want them, and not if you don't.

> > (I have defined ilog2(n) as returning an undefined value if n < 1).
> 
> Undefined values are bad. Could you produce a runtime error instead?

But if I make it clear to the user of ilog(n) that *they* are expected to check
the parameters, then it shouldn't be a problem.  Quite often there's no need to
check because n can't be out of range without other things breaking.

Doing enforced runtime error checking slows things down and is frequently
unnecessary.  Actually, in a way it's better to let the caller do it as they
know whether it's necessary, and can probably find a better place to do it.

Consider div64(): that might not produce an error if the divisor is 0.  And
then there's __ffs() which is explicitly stated as producing an undefined
result if no bits are set in its argument.

> > > >  (5) fls() and fls64() can't be used to initialise a variable at compile
> > > >      time, ilog2() can.
> > > 
> > > Well that is the same issue as (4).
> > 
> > Not quite.  I think (4) might be sufficiently achievable with an inline
> > function, but (5) is definitely not.
> 
> With the appropriate per arch modification of fls() this should also be 
> possible within the existing framework.

True, but I suspect that's going to be less required of fls() and co.

David