Subject: Re: [PATCH] remove powerpc bitops infavor of existing generic bitops

From: Jon Mason <[email-masked]>
Date: Mon, 15 May 2006 15:38:15 -0500

On Mon, May 15, 2006 at 02:56:13PM -0500, jschopp wrote:
> >There already exists a big endian safe bitops implementation in
> >lib/find_next_bit.c.  The code in it is 90%+ common with the powerpc
> >specific version, so the powerpc version is redundant.  This patch
> >makes the necessary changes to use the generic bitops in powerpc, and
> >removes the powerpc specific version.
> 
> I like generic as much as the next guy, but I'm also a big fan of fast 
> bitops.  And the function below is fast.  You'll have to explain to me how 
> the generic code is going to find the first zero as fast without explicit 
> calls to ppc assembly.
> 
> >-static inline unsigned int ext2_ilog2(unsigned int x)
> >-{
> >-	int lz;
> >-
> >-	asm("cntlzw %0,%1": "=r"(lz):"r"(x));
> >-	return 31 - lz;
> >-}

Ah but here's the trick, there is the same explicit call to ppc
assembly.  The only function in the file removed is this one you pointed
out, and the only caller of this function is ext2_ffz.  And the only
user of ext2_ffz is find_next_zero_le_bit.  

Now the generic code is very similar to the file removed (`diff -Narup
arch/powerpc/lib/bitops.c lib/find_next_bit.c` to see for yourself).  In
the same place where ext2_ffz is called, ffz is called in the generic
code.  Now if we look at the definition of ffz in
include/asm-powerpc/bitops.h, we see it calls __ilog2 of that same file.
__ilog2 is defined as: 

static __inline__ int __ilog2(unsigned long x)
{
        int lz;

        asm (PPC_CNTLZL "%0,%1" : "=r" (lz) : "r" (x));
        return BITS_PER_LONG - 1 - lz;
}

So, its really the same code :)

Thanks,
Jon