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

[David Howells]

Christoph Lameter <clameter@xxxxxxx> wrote:

> So this is a case for a wrapper.

But it _is_ a wrapper - but the arch gets to decide around what.  We fall back
to fls() or fls64() if no other inspiration strikes or is available, but in
some cases we can do better.

> Good stuff. I have always wanted that. The wrapper could check for a 
> constant.

That's the main reason I put it in a common file.  The const-case wrapper is
large and probably best not repeated.

> >  (3) ilog2(n) != fls(n)
> > 
> >      This means that the asm-optimised version for one might be less
> >      optimal for the other (for example, ilog2() produces an undefined
> >      result if n <= 1, fls() must return 0).
> 
> Ok these are boundary checks that are easily coded around. Some 
> variations on fls even exist that also do various flavors of end case 
> handling.

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).

So fls() is 5 insns and __ilog2_u32() is 1 because of the requirement that
fls() must return 0 on a zero input value.  Similarly fls64() is 14 vs 8 for
__ilog2_u64().

(I have defined ilog2(n) as returning an undefined value if n < 1).

> >  (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.

David
-
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/