* [PATCH] remove powerpc bitops infavor of existing generic bitops
@ 2006-05-15 18:01 Jon Mason
2006-05-15 19:56 ` jschopp
2006-05-19 5:00 ` Paul Mackerras
0 siblings, 2 replies; 6+ messages in thread
From: Jon Mason @ 2006-05-15 18:01 UTC (permalink / raw)
To: linuxppc-dev
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.
Thanks,
Jon
Signed-off-by: Jon Mason <jdmason@us.ibm.com>
diff -r bfccde0f7221 arch/powerpc/Kconfig
--- a/arch/powerpc/Kconfig Sat May 13 08:42:09 2006 +0700
+++ b/arch/powerpc/Kconfig Mon May 15 17:30:46 2006 +0000
@@ -42,6 +42,10 @@ config GENERIC_HWEIGHT
default y
config GENERIC_CALIBRATE_DELAY
+ bool
+ default y
+
+config GENERIC_FIND_NEXT_BIT
bool
default y
diff -r bfccde0f7221 arch/powerpc/lib/Makefile
--- a/arch/powerpc/lib/Makefile Sat May 13 08:42:09 2006 +0700
+++ b/arch/powerpc/lib/Makefile Mon May 15 17:30:46 2006 +0000
@@ -7,7 +7,6 @@ obj-$(CONFIG_PPC32) += div64.o copy_32.o
obj-$(CONFIG_PPC32) += div64.o copy_32.o checksum_32.o
endif
-obj-y += bitops.o
obj-$(CONFIG_PPC64) += checksum_64.o copypage_64.o copyuser_64.o \
memcpy_64.o usercopy_64.o mem_64.o string.o \
strcase.o
diff -r bfccde0f7221 include/asm-powerpc/bitops.h
--- a/include/asm-powerpc/bitops.h Sat May 13 08:42:09 2006 +0700
+++ b/include/asm-powerpc/bitops.h Mon May 15 17:30:46 2006 +0000
@@ -288,8 +288,8 @@ static __inline__ int test_le_bit(unsign
#define __test_and_clear_le_bit(nr, addr) \
__test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-#define find_first_zero_le_bit(addr, size) find_next_zero_le_bit((addr), (size), 0)
-unsigned long find_next_zero_le_bit(const unsigned long *addr,
+#define find_first_zero_le_bit(addr, size) generic_find_next_zero_le_bit((addr), (size), 0)
+unsigned long generic_find_next_zero_le_bit(const unsigned long *addr,
unsigned long size, unsigned long offset);
/* Bitmap functions for the ext2 filesystem */
@@ -309,7 +309,7 @@ unsigned long find_next_zero_le_bit(cons
#define ext2_find_first_zero_bit(addr, size) \
find_first_zero_le_bit((unsigned long*)addr, size)
#define ext2_find_next_zero_bit(addr, size, off) \
- find_next_zero_le_bit((unsigned long*)addr, size, off)
+ generic_find_next_zero_le_bit((unsigned long*)addr, size, off)
/* Bitmap functions for the minix filesystem. */
diff -r bfccde0f7221 arch/powerpc/lib/bitops.c
--- a/arch/powerpc/lib/bitops.c Sat May 13 08:42:09 2006 +0700
+++ /dev/null Thu Jan 01 00:00:00 1970 +0000
@@ -1,150 +0,0 @@
-#include <linux/types.h>
-#include <linux/module.h>
-#include <asm/byteorder.h>
-#include <asm/bitops.h>
-
-/**
- * find_next_bit - find the next set bit in a memory region
- * @addr: The address to base the search on
- * @offset: The bitnumber to start searching at
- * @size: The maximum size to search
- */
-unsigned long find_next_bit(const unsigned long *addr, unsigned long size,
- unsigned long offset)
-{
- const unsigned long *p = addr + BITOP_WORD(offset);
- unsigned long result = offset & ~(BITS_PER_LONG-1);
- unsigned long tmp;
-
- if (offset >= size)
- return size;
- size -= result;
- offset %= BITS_PER_LONG;
- if (offset) {
- tmp = *(p++);
- tmp &= (~0UL << offset);
- if (size < BITS_PER_LONG)
- goto found_first;
- if (tmp)
- goto found_middle;
- size -= BITS_PER_LONG;
- result += BITS_PER_LONG;
- }
- while (size & ~(BITS_PER_LONG-1)) {
- if ((tmp = *(p++)))
- goto found_middle;
- result += BITS_PER_LONG;
- size -= BITS_PER_LONG;
- }
- if (!size)
- return result;
- tmp = *p;
-
-found_first:
- tmp &= (~0UL >> (BITS_PER_LONG - size));
- if (tmp == 0UL) /* Are any bits set? */
- return result + size; /* Nope. */
-found_middle:
- return result + __ffs(tmp);
-}
-EXPORT_SYMBOL(find_next_bit);
-
-/*
- * This implementation of find_{first,next}_zero_bit was stolen from
- * Linus' asm-alpha/bitops.h.
- */
-unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size,
- unsigned long offset)
-{
- const unsigned long *p = addr + BITOP_WORD(offset);
- unsigned long result = offset & ~(BITS_PER_LONG-1);
- unsigned long tmp;
-
- if (offset >= size)
- return size;
- size -= result;
- offset %= BITS_PER_LONG;
- if (offset) {
- tmp = *(p++);
- tmp |= ~0UL >> (BITS_PER_LONG - offset);
- if (size < BITS_PER_LONG)
- goto found_first;
- if (~tmp)
- goto found_middle;
- size -= BITS_PER_LONG;
- result += BITS_PER_LONG;
- }
- while (size & ~(BITS_PER_LONG-1)) {
- if (~(tmp = *(p++)))
- goto found_middle;
- result += BITS_PER_LONG;
- size -= BITS_PER_LONG;
- }
- if (!size)
- return result;
- tmp = *p;
-
-found_first:
- tmp |= ~0UL << size;
- if (tmp == ~0UL) /* Are any bits zero? */
- return result + size; /* Nope. */
-found_middle:
- return result + ffz(tmp);
-}
-EXPORT_SYMBOL(find_next_zero_bit);
-
-static inline unsigned int ext2_ilog2(unsigned int x)
-{
- int lz;
-
- asm("cntlzw %0,%1": "=r"(lz):"r"(x));
- return 31 - lz;
-}
-
-static inline unsigned int ext2_ffz(unsigned int x)
-{
- u32 rc;
- if ((x = ~x) == 0)
- return 32;
- rc = ext2_ilog2(x & -x);
- return rc;
-}
-
-unsigned long find_next_zero_le_bit(const unsigned long *addr,
- unsigned long size, unsigned long offset)
-{
- const unsigned int *p = ((const unsigned int *)addr) + (offset >> 5);
- unsigned int result = offset & ~31;
- unsigned int tmp;
-
- if (offset >= size)
- return size;
- size -= result;
- offset &= 31;
- if (offset) {
- tmp = cpu_to_le32p(p++);
- tmp |= ~0U >> (32 - offset); /* bug or feature ? */
- if (size < 32)
- goto found_first;
- if (tmp != ~0)
- goto found_middle;
- size -= 32;
- result += 32;
- }
- while (size >= 32) {
- if ((tmp = cpu_to_le32p(p++)) != ~0)
- goto found_middle;
- result += 32;
- size -= 32;
- }
- if (!size)
- return result;
- tmp = cpu_to_le32p(p);
-found_first:
- tmp |= ~0 << size;
- if (tmp == ~0) /* Are any bits zero? */
- return result + size; /* Nope. */
-found_middle:
- return result + ext2_ffz(tmp);
-}
-EXPORT_SYMBOL(find_next_zero_le_bit);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] remove powerpc bitops infavor of existing generic bitops
2006-05-15 18:01 [PATCH] remove powerpc bitops infavor of existing generic bitops Jon Mason
@ 2006-05-15 19:56 ` jschopp
2006-05-15 20:38 ` Jon Mason
2006-05-19 5:00 ` Paul Mackerras
1 sibling, 1 reply; 6+ messages in thread
From: jschopp @ 2006-05-15 19:56 UTC (permalink / raw)
To: Jon Mason; +Cc: linuxppc-dev
> 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;
> -}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] remove powerpc bitops infavor of existing generic bitops
2006-05-15 19:56 ` jschopp
@ 2006-05-15 20:38 ` Jon Mason
2006-05-15 20:42 ` jschopp
0 siblings, 1 reply; 6+ messages in thread
From: Jon Mason @ 2006-05-15 20:38 UTC (permalink / raw)
To: jschopp; +Cc: linuxppc-dev
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] remove powerpc bitops infavor of existing generic bitops
2006-05-15 20:38 ` Jon Mason
@ 2006-05-15 20:42 ` jschopp
0 siblings, 0 replies; 6+ messages in thread
From: jschopp @ 2006-05-15 20:42 UTC (permalink / raw)
To: Jon Mason; +Cc: linuxppc-dev
> 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 :)
Good explination. I'm convinced, so for what it's worth:
Acked-by: Joel Schopp <jschopp@austin.ibm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] remove powerpc bitops infavor of existing generic bitops
2006-05-15 18:01 [PATCH] remove powerpc bitops infavor of existing generic bitops Jon Mason
2006-05-15 19:56 ` jschopp
@ 2006-05-19 5:00 ` Paul Mackerras
2006-05-19 20:35 ` Jon Mason
1 sibling, 1 reply; 6+ messages in thread
From: Paul Mackerras @ 2006-05-19 5:00 UTC (permalink / raw)
To: Jon Mason; +Cc: linuxppc-dev
Jon Mason writes:
> 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.
This patch breaks ARCH=ppc builds. Please resubmit with the necessary
changes to arch/ppc/Kconfig as well.
Paul.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] remove powerpc bitops infavor of existing generic bitops
2006-05-19 5:00 ` Paul Mackerras
@ 2006-05-19 20:35 ` Jon Mason
0 siblings, 0 replies; 6+ messages in thread
From: Jon Mason @ 2006-05-19 20:35 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
On Fri, May 19, 2006 at 03:00:41PM +1000, Paul Mackerras wrote:
> Jon Mason writes:
>
> > 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.
>
> This patch breaks ARCH=ppc builds. Please resubmit with the necessary
> changes to arch/ppc/Kconfig as well.
>
> Paul.
Sorry. Here is the patch with the necessary correction.
Thanks,
Jon
Signed-off-by: Jon Mason <jdmason@us.ibm.com>
diff -r a89fd2f124df arch/powerpc/Kconfig
--- a/arch/powerpc/Kconfig Fri May 19 16:17:18 2006 +0700
+++ b/arch/powerpc/Kconfig Fri May 19 15:34:39 2006 -0500
@@ -42,6 +42,10 @@ config GENERIC_HWEIGHT
default y
config GENERIC_CALIBRATE_DELAY
+ bool
+ default y
+
+config GENERIC_FIND_NEXT_BIT
bool
default y
diff -r a89fd2f124df arch/powerpc/lib/Makefile
--- a/arch/powerpc/lib/Makefile Fri May 19 16:17:18 2006 +0700
+++ b/arch/powerpc/lib/Makefile Fri May 19 15:34:39 2006 -0500
@@ -7,7 +7,6 @@ obj-$(CONFIG_PPC32) += div64.o copy_32.o
obj-$(CONFIG_PPC32) += div64.o copy_32.o checksum_32.o
endif
-obj-y += bitops.o
obj-$(CONFIG_PPC64) += checksum_64.o copypage_64.o copyuser_64.o \
memcpy_64.o usercopy_64.o mem_64.o string.o \
strcase.o
diff -r a89fd2f124df arch/ppc/Kconfig
--- a/arch/ppc/Kconfig Fri May 19 16:17:18 2006 +0700
+++ b/arch/ppc/Kconfig Fri May 19 15:34:39 2006 -0500
@@ -37,6 +37,10 @@ config PPC32
# All PPCs use generic nvram driver through ppc_md
config GENERIC_NVRAM
+ bool
+ default y
+
+config GENERIC_FIND_NEXT_BIT
bool
default y
diff -r a89fd2f124df include/asm-powerpc/bitops.h
--- a/include/asm-powerpc/bitops.h Fri May 19 16:17:18 2006 +0700
+++ b/include/asm-powerpc/bitops.h Fri May 19 15:34:39 2006 -0500
@@ -288,8 +288,8 @@ static __inline__ int test_le_bit(unsign
#define __test_and_clear_le_bit(nr, addr) \
__test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-#define find_first_zero_le_bit(addr, size) find_next_zero_le_bit((addr), (size), 0)
-unsigned long find_next_zero_le_bit(const unsigned long *addr,
+#define find_first_zero_le_bit(addr, size) generic_find_next_zero_le_bit((addr), (size), 0)
+unsigned long generic_find_next_zero_le_bit(const unsigned long *addr,
unsigned long size, unsigned long offset);
/* Bitmap functions for the ext2 filesystem */
@@ -309,7 +309,7 @@ unsigned long find_next_zero_le_bit(cons
#define ext2_find_first_zero_bit(addr, size) \
find_first_zero_le_bit((unsigned long*)addr, size)
#define ext2_find_next_zero_bit(addr, size, off) \
- find_next_zero_le_bit((unsigned long*)addr, size, off)
+ generic_find_next_zero_le_bit((unsigned long*)addr, size, off)
/* Bitmap functions for the minix filesystem. */
diff -r a89fd2f124df arch/powerpc/lib/bitops.c
--- a/arch/powerpc/lib/bitops.c Fri May 19 16:17:18 2006 +0700
+++ /dev/null Thu Jan 1 00:00:00 1970 +0000
@@ -1,150 +0,0 @@
-#include <linux/types.h>
-#include <linux/module.h>
-#include <asm/byteorder.h>
-#include <asm/bitops.h>
-
-/**
- * find_next_bit - find the next set bit in a memory region
- * @addr: The address to base the search on
- * @offset: The bitnumber to start searching at
- * @size: The maximum size to search
- */
-unsigned long find_next_bit(const unsigned long *addr, unsigned long size,
- unsigned long offset)
-{
- const unsigned long *p = addr + BITOP_WORD(offset);
- unsigned long result = offset & ~(BITS_PER_LONG-1);
- unsigned long tmp;
-
- if (offset >= size)
- return size;
- size -= result;
- offset %= BITS_PER_LONG;
- if (offset) {
- tmp = *(p++);
- tmp &= (~0UL << offset);
- if (size < BITS_PER_LONG)
- goto found_first;
- if (tmp)
- goto found_middle;
- size -= BITS_PER_LONG;
- result += BITS_PER_LONG;
- }
- while (size & ~(BITS_PER_LONG-1)) {
- if ((tmp = *(p++)))
- goto found_middle;
- result += BITS_PER_LONG;
- size -= BITS_PER_LONG;
- }
- if (!size)
- return result;
- tmp = *p;
-
-found_first:
- tmp &= (~0UL >> (BITS_PER_LONG - size));
- if (tmp == 0UL) /* Are any bits set? */
- return result + size; /* Nope. */
-found_middle:
- return result + __ffs(tmp);
-}
-EXPORT_SYMBOL(find_next_bit);
-
-/*
- * This implementation of find_{first,next}_zero_bit was stolen from
- * Linus' asm-alpha/bitops.h.
- */
-unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size,
- unsigned long offset)
-{
- const unsigned long *p = addr + BITOP_WORD(offset);
- unsigned long result = offset & ~(BITS_PER_LONG-1);
- unsigned long tmp;
-
- if (offset >= size)
- return size;
- size -= result;
- offset %= BITS_PER_LONG;
- if (offset) {
- tmp = *(p++);
- tmp |= ~0UL >> (BITS_PER_LONG - offset);
- if (size < BITS_PER_LONG)
- goto found_first;
- if (~tmp)
- goto found_middle;
- size -= BITS_PER_LONG;
- result += BITS_PER_LONG;
- }
- while (size & ~(BITS_PER_LONG-1)) {
- if (~(tmp = *(p++)))
- goto found_middle;
- result += BITS_PER_LONG;
- size -= BITS_PER_LONG;
- }
- if (!size)
- return result;
- tmp = *p;
-
-found_first:
- tmp |= ~0UL << size;
- if (tmp == ~0UL) /* Are any bits zero? */
- return result + size; /* Nope. */
-found_middle:
- return result + ffz(tmp);
-}
-EXPORT_SYMBOL(find_next_zero_bit);
-
-static inline unsigned int ext2_ilog2(unsigned int x)
-{
- int lz;
-
- asm("cntlzw %0,%1": "=r"(lz):"r"(x));
- return 31 - lz;
-}
-
-static inline unsigned int ext2_ffz(unsigned int x)
-{
- u32 rc;
- if ((x = ~x) == 0)
- return 32;
- rc = ext2_ilog2(x & -x);
- return rc;
-}
-
-unsigned long find_next_zero_le_bit(const unsigned long *addr,
- unsigned long size, unsigned long offset)
-{
- const unsigned int *p = ((const unsigned int *)addr) + (offset >> 5);
- unsigned int result = offset & ~31;
- unsigned int tmp;
-
- if (offset >= size)
- return size;
- size -= result;
- offset &= 31;
- if (offset) {
- tmp = cpu_to_le32p(p++);
- tmp |= ~0U >> (32 - offset); /* bug or feature ? */
- if (size < 32)
- goto found_first;
- if (tmp != ~0)
- goto found_middle;
- size -= 32;
- result += 32;
- }
- while (size >= 32) {
- if ((tmp = cpu_to_le32p(p++)) != ~0)
- goto found_middle;
- result += 32;
- size -= 32;
- }
- if (!size)
- return result;
- tmp = cpu_to_le32p(p);
-found_first:
- tmp |= ~0 << size;
- if (tmp == ~0) /* Are any bits zero? */
- return result + size; /* Nope. */
-found_middle:
- return result + ext2_ffz(tmp);
-}
-EXPORT_SYMBOL(find_next_zero_le_bit);
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-05-19 20:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-05-15 18:01 [PATCH] remove powerpc bitops infavor of existing generic bitops Jon Mason
2006-05-15 19:56 ` jschopp
2006-05-15 20:38 ` Jon Mason
2006-05-15 20:42 ` jschopp
2006-05-19 5:00 ` Paul Mackerras
2006-05-19 20:35 ` Jon Mason
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).