All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] core/hweight changes for v2.6.35
@ 2010-05-17 21:21 Ingo Molnar
  2010-05-17 21:46 ` Linus Torvalds
  2010-08-13 21:42 ` [GIT PULL] " Mike Frysinger
  0 siblings, 2 replies; 12+ messages in thread
From: Ingo Molnar @ 2010-05-17 21:21 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, H. Peter Anvin, Borislav Petkov, Peter Zijlstra,
	Thomas Gleixner, Andrew Morton

Linus,

Please pull the latest core-hweight-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core-hweight-for-linus

 Thanks,

	Ingo

------------------>
Borislav Petkov (2):
      x86: Add optimized popcnt variants
      arch, hweight: Fix compilation errors

Peter Zijlstra (1):
      bitops: Optimize hweight() by making use of compile-time evaluation


 arch/alpha/include/asm/bitops.h            |   18 +++++----
 arch/ia64/include/asm/bitops.h             |   11 +++--
 arch/sparc/include/asm/bitops_64.h         |   11 +++--
 arch/x86/Kconfig                           |    5 ++
 arch/x86/include/asm/alternative.h         |    9 +++-
 arch/x86/include/asm/arch_hweight.h        |   59 ++++++++++++++++++++++++++++
 arch/x86/include/asm/bitops.h              |    4 +-
 include/asm-generic/bitops/arch_hweight.h  |   25 ++++++++++++
 include/asm-generic/bitops/const_hweight.h |   42 ++++++++++++++++++++
 include/asm-generic/bitops/hweight.h       |    8 +---
 include/linux/bitops.h                     |   30 ++------------
 lib/Makefile                               |    3 +
 lib/hweight.c                              |   19 +++++----
 scripts/Makefile.lib                       |    4 ++
 14 files changed, 186 insertions(+), 62 deletions(-)
 create mode 100644 arch/x86/include/asm/arch_hweight.h
 create mode 100644 include/asm-generic/bitops/arch_hweight.h
 create mode 100644 include/asm-generic/bitops/const_hweight.h

diff --git a/arch/alpha/include/asm/bitops.h b/arch/alpha/include/asm/bitops.h
index 15f3ae2..296da1d 100644
--- a/arch/alpha/include/asm/bitops.h
+++ b/arch/alpha/include/asm/bitops.h
@@ -405,29 +405,31 @@ static inline int fls(int x)
 
 #if defined(CONFIG_ALPHA_EV6) && defined(CONFIG_ALPHA_EV67)
 /* Whee.  EV67 can calculate it directly.  */
-static inline unsigned long hweight64(unsigned long w)
+static inline unsigned long __arch_hweight64(unsigned long w)
 {
 	return __kernel_ctpop(w);
 }
 
-static inline unsigned int hweight32(unsigned int w)
+static inline unsigned int __arch_weight32(unsigned int w)
 {
-	return hweight64(w);
+	return __arch_hweight64(w);
 }
 
-static inline unsigned int hweight16(unsigned int w)
+static inline unsigned int __arch_hweight16(unsigned int w)
 {
-	return hweight64(w & 0xffff);
+	return __arch_hweight64(w & 0xffff);
 }
 
-static inline unsigned int hweight8(unsigned int w)
+static inline unsigned int __arch_hweight8(unsigned int w)
 {
-	return hweight64(w & 0xff);
+	return __arch_hweight64(w & 0xff);
 }
 #else
-#include <asm-generic/bitops/hweight.h>
+#include <asm-generic/bitops/arch_hweight.h>
 #endif
 
+#include <asm-generic/bitops/const_hweight.h>
+
 #endif /* __KERNEL__ */
 
 #include <asm-generic/bitops/find.h>
diff --git a/arch/ia64/include/asm/bitops.h b/arch/ia64/include/asm/bitops.h
index 6ebc229..9da3df6 100644
--- a/arch/ia64/include/asm/bitops.h
+++ b/arch/ia64/include/asm/bitops.h
@@ -437,17 +437,18 @@ __fls (unsigned long x)
  * hweightN: returns the hamming weight (i.e. the number
  * of bits set) of a N-bit word
  */
-static __inline__ unsigned long
-hweight64 (unsigned long x)
+static __inline__ unsigned long __arch_hweight64(unsigned long x)
 {
 	unsigned long result;
 	result = ia64_popcnt(x);
 	return result;
 }
 
-#define hweight32(x)	(unsigned int) hweight64((x) & 0xfffffffful)
-#define hweight16(x)	(unsigned int) hweight64((x) & 0xfffful)
-#define hweight8(x)	(unsigned int) hweight64((x) & 0xfful)
+#define __arch_hweight32(x) ((unsigned int) __arch_hweight64((x) & 0xfffffffful))
+#define __arch_hweight16(x) ((unsigned int) __arch_hweight64((x) & 0xfffful))
+#define __arch_hweight8(x)  ((unsigned int) __arch_hweight64((x) & 0xfful))
+
+#include <asm-generic/bitops/const_hweight.h>
 
 #endif /* __KERNEL__ */
 
diff --git a/arch/sparc/include/asm/bitops_64.h b/arch/sparc/include/asm/bitops_64.h
index e72ac9c..766121a 100644
--- a/arch/sparc/include/asm/bitops_64.h
+++ b/arch/sparc/include/asm/bitops_64.h
@@ -44,7 +44,7 @@ extern void change_bit(unsigned long nr, volatile unsigned long *addr);
 
 #ifdef ULTRA_HAS_POPULATION_COUNT
 
-static inline unsigned int hweight64(unsigned long w)
+static inline unsigned int __arch_hweight64(unsigned long w)
 {
 	unsigned int res;
 
@@ -52,7 +52,7 @@ static inline unsigned int hweight64(unsigned long w)
 	return res;
 }
 
-static inline unsigned int hweight32(unsigned int w)
+static inline unsigned int __arch_hweight32(unsigned int w)
 {
 	unsigned int res;
 
@@ -60,7 +60,7 @@ static inline unsigned int hweight32(unsigned int w)
 	return res;
 }
 
-static inline unsigned int hweight16(unsigned int w)
+static inline unsigned int __arch_hweight16(unsigned int w)
 {
 	unsigned int res;
 
@@ -68,7 +68,7 @@ static inline unsigned int hweight16(unsigned int w)
 	return res;
 }
 
-static inline unsigned int hweight8(unsigned int w)
+static inline unsigned int __arch_hweight8(unsigned int w)
 {
 	unsigned int res;
 
@@ -78,9 +78,10 @@ static inline unsigned int hweight8(unsigned int w)
 
 #else
 
-#include <asm-generic/bitops/hweight.h>
+#include <asm-generic/bitops/arch_hweight.h>
 
 #endif
+#include <asm-generic/bitops/const_hweight.h>
 #include <asm-generic/bitops/lock.h>
 #endif /* __KERNEL__ */
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0eacb1f..89d8c54 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -238,6 +238,11 @@ config X86_32_LAZY_GS
 	def_bool y
 	depends on X86_32 && !CC_STACKPROTECTOR
 
+config ARCH_HWEIGHT_CFLAGS
+	string
+	default "-fcall-saved-ecx -fcall-saved-edx" if X86_32
+	default "-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fcall-saved-r11" if X86_64
+
 config KTIME_SCALAR
 	def_bool X86_32
 source "init/Kconfig"
diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index b09ec55..67dae51 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -39,9 +39,6 @@
 #define LOCK_PREFIX ""
 #endif
 
-/* This must be included *after* the definition of LOCK_PREFIX */
-#include <asm/cpufeature.h>
-
 struct alt_instr {
 	u8 *instr;		/* original instruction */
 	u8 *replacement;
@@ -96,6 +93,12 @@ static inline int alternatives_text_reserved(void *start, void *end)
       ".previous"
 
 /*
+ * This must be included *after* the definition of ALTERNATIVE due to
+ * <asm/arch_hweight.h>
+ */
+#include <asm/cpufeature.h>
+
+/*
  * Alternative instructions for different CPU types or capabilities.
  *
  * This allows to use optimized instructions even on generic binary
diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h
new file mode 100644
index 0000000..d1fc3c2
--- /dev/null
+++ b/arch/x86/include/asm/arch_hweight.h
@@ -0,0 +1,59 @@
+#ifndef _ASM_X86_HWEIGHT_H
+#define _ASM_X86_HWEIGHT_H
+
+#ifdef CONFIG_64BIT
+/* popcnt %rdi, %rax */
+#define POPCNT ".byte 0xf3,0x48,0x0f,0xb8,0xc7"
+#define REG_IN "D"
+#define REG_OUT "a"
+#else
+/* popcnt %eax, %eax */
+#define POPCNT ".byte 0xf3,0x0f,0xb8,0xc0"
+#define REG_IN "a"
+#define REG_OUT "a"
+#endif
+
+/*
+ * __sw_hweightXX are called from within the alternatives below
+ * and callee-clobbered registers need to be taken care of. See
+ * ARCH_HWEIGHT_CFLAGS in <arch/x86/Kconfig> for the respective
+ * compiler switches.
+ */
+static inline unsigned int __arch_hweight32(unsigned int w)
+{
+	unsigned int res = 0;
+
+	asm (ALTERNATIVE("call __sw_hweight32", POPCNT, X86_FEATURE_POPCNT)
+		     : "="REG_OUT (res)
+		     : REG_IN (w));
+
+	return res;
+}
+
+static inline unsigned int __arch_hweight16(unsigned int w)
+{
+	return __arch_hweight32(w & 0xffff);
+}
+
+static inline unsigned int __arch_hweight8(unsigned int w)
+{
+	return __arch_hweight32(w & 0xff);
+}
+
+static inline unsigned long __arch_hweight64(__u64 w)
+{
+	unsigned long res = 0;
+
+#ifdef CONFIG_X86_32
+	return  __arch_hweight32((u32)w) +
+		__arch_hweight32((u32)(w >> 32));
+#else
+	asm (ALTERNATIVE("call __sw_hweight64", POPCNT, X86_FEATURE_POPCNT)
+		     : "="REG_OUT (res)
+		     : REG_IN (w));
+#endif /* CONFIG_X86_32 */
+
+	return res;
+}
+
+#endif
diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index 02b47a6..545776e 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -444,7 +444,9 @@ static inline int fls(int x)
 
 #define ARCH_HAS_FAST_MULTIPLIER 1
 
-#include <asm-generic/bitops/hweight.h>
+#include <asm/arch_hweight.h>
+
+#include <asm-generic/bitops/const_hweight.h>
 
 #endif /* __KERNEL__ */
 
diff --git a/include/asm-generic/bitops/arch_hweight.h b/include/asm-generic/bitops/arch_hweight.h
new file mode 100644
index 0000000..6a211f4
--- /dev/null
+++ b/include/asm-generic/bitops/arch_hweight.h
@@ -0,0 +1,25 @@
+#ifndef _ASM_GENERIC_BITOPS_ARCH_HWEIGHT_H_
+#define _ASM_GENERIC_BITOPS_ARCH_HWEIGHT_H_
+
+#include <asm/types.h>
+
+static inline unsigned int __arch_hweight32(unsigned int w)
+{
+	return __sw_hweight32(w);
+}
+
+static inline unsigned int __arch_hweight16(unsigned int w)
+{
+	return __sw_hweight16(w);
+}
+
+static inline unsigned int __arch_hweight8(unsigned int w)
+{
+	return __sw_hweight8(w);
+}
+
+static inline unsigned long __arch_hweight64(__u64 w)
+{
+	return __sw_hweight64(w);
+}
+#endif /* _ASM_GENERIC_BITOPS_HWEIGHT_H_ */
diff --git a/include/asm-generic/bitops/const_hweight.h b/include/asm-generic/bitops/const_hweight.h
new file mode 100644
index 0000000..fa2a50b
--- /dev/null
+++ b/include/asm-generic/bitops/const_hweight.h
@@ -0,0 +1,42 @@
+#ifndef _ASM_GENERIC_BITOPS_CONST_HWEIGHT_H_
+#define _ASM_GENERIC_BITOPS_CONST_HWEIGHT_H_
+
+/*
+ * Compile time versions of __arch_hweightN()
+ */
+#define __const_hweight8(w)		\
+      (	(!!((w) & (1ULL << 0))) +	\
+	(!!((w) & (1ULL << 1))) +	\
+	(!!((w) & (1ULL << 2))) +	\
+	(!!((w) & (1ULL << 3))) +	\
+	(!!((w) & (1ULL << 4))) +	\
+	(!!((w) & (1ULL << 5))) +	\
+	(!!((w) & (1ULL << 6))) +	\
+	(!!((w) & (1ULL << 7)))	)
+
+#define __const_hweight16(w) (__const_hweight8(w)  + __const_hweight8((w)  >> 8 ))
+#define __const_hweight32(w) (__const_hweight16(w) + __const_hweight16((w) >> 16))
+#define __const_hweight64(w) (__const_hweight32(w) + __const_hweight32((w) >> 32))
+
+/*
+ * Generic interface.
+ */
+#define hweight8(w)  (__builtin_constant_p(w) ? __const_hweight8(w)  : __arch_hweight8(w))
+#define hweight16(w) (__builtin_constant_p(w) ? __const_hweight16(w) : __arch_hweight16(w))
+#define hweight32(w) (__builtin_constant_p(w) ? __const_hweight32(w) : __arch_hweight32(w))
+#define hweight64(w) (__builtin_constant_p(w) ? __const_hweight64(w) : __arch_hweight64(w))
+
+/*
+ * Interface for known constant arguments
+ */
+#define HWEIGHT8(w)  (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight8(w))
+#define HWEIGHT16(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight16(w))
+#define HWEIGHT32(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight32(w))
+#define HWEIGHT64(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight64(w))
+
+/*
+ * Type invariant interface to the compile time constant hweight functions.
+ */
+#define HWEIGHT(w)   HWEIGHT64((u64)w)
+
+#endif /* _ASM_GENERIC_BITOPS_CONST_HWEIGHT_H_ */
diff --git a/include/asm-generic/bitops/hweight.h b/include/asm-generic/bitops/hweight.h
index fbbc383..a94d651 100644
--- a/include/asm-generic/bitops/hweight.h
+++ b/include/asm-generic/bitops/hweight.h
@@ -1,11 +1,7 @@
 #ifndef _ASM_GENERIC_BITOPS_HWEIGHT_H_
 #define _ASM_GENERIC_BITOPS_HWEIGHT_H_
 
-#include <asm/types.h>
-
-extern unsigned int hweight32(unsigned int w);
-extern unsigned int hweight16(unsigned int w);
-extern unsigned int hweight8(unsigned int w);
-extern unsigned long hweight64(__u64 w);
+#include <asm-generic/bitops/arch_hweight.h>
+#include <asm-generic/bitops/const_hweight.h>
 
 #endif /* _ASM_GENERIC_BITOPS_HWEIGHT_H_ */
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index b793898..26caa60 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -10,6 +10,11 @@
 #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 #endif
 
+extern unsigned int __sw_hweight8(unsigned int w);
+extern unsigned int __sw_hweight16(unsigned int w);
+extern unsigned int __sw_hweight32(unsigned int w);
+extern unsigned long __sw_hweight64(__u64 w);
+
 /*
  * Include this here because some architectures need generic_ffs/fls in
  * scope
@@ -47,31 +52,6 @@ static inline unsigned long hweight_long(unsigned long w)
 	return sizeof(w) == 4 ? hweight32(w) : hweight64(w);
 }
 
-/*
- * Clearly slow versions of the hweightN() functions, their benefit is
- * of course compile time evaluation of constant arguments.
- */
-#define HWEIGHT8(w)					\
-      (	BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) +	\
-	(!!((w) & (1ULL << 0))) +			\
-	(!!((w) & (1ULL << 1))) +			\
-	(!!((w) & (1ULL << 2))) +			\
-	(!!((w) & (1ULL << 3))) +			\
-	(!!((w) & (1ULL << 4))) +			\
-	(!!((w) & (1ULL << 5))) +			\
-	(!!((w) & (1ULL << 6))) +			\
-	(!!((w) & (1ULL << 7)))	)
-
-#define HWEIGHT16(w) (HWEIGHT8(w)  + HWEIGHT8((w) >> 8))
-#define HWEIGHT32(w) (HWEIGHT16(w) + HWEIGHT16((w) >> 16))
-#define HWEIGHT64(w) (HWEIGHT32(w) + HWEIGHT32((w) >> 32))
-
-/*
- * Type invariant version that simply casts things to the
- * largest type.
- */
-#define HWEIGHT(w)   HWEIGHT64((u64)(w))
-
 /**
  * rol32 - rotate a 32-bit value left
  * @word: value to rotate
diff --git a/lib/Makefile b/lib/Makefile
index 2e152ae..abe63a8 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -39,7 +39,10 @@ lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
 lib-$(CONFIG_GENERIC_FIND_FIRST_BIT) += find_next_bit.o
 lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find_next_bit.o
 obj-$(CONFIG_GENERIC_FIND_LAST_BIT) += find_last_bit.o
+
+CFLAGS_hweight.o = $(subst $(quote),,$(CONFIG_ARCH_HWEIGHT_CFLAGS))
 obj-$(CONFIG_GENERIC_HWEIGHT) += hweight.o
+
 obj-$(CONFIG_LOCK_KERNEL) += kernel_lock.o
 obj-$(CONFIG_BTREE) += btree.o
 obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o
diff --git a/lib/hweight.c b/lib/hweight.c
index 63ee4eb..3c79d50 100644
--- a/lib/hweight.c
+++ b/lib/hweight.c
@@ -9,7 +9,7 @@
  * The Hamming Weight of a number is the total number of bits set in it.
  */
 
-unsigned int hweight32(unsigned int w)
+unsigned int __sw_hweight32(unsigned int w)
 {
 #ifdef ARCH_HAS_FAST_MULTIPLIER
 	w -= (w >> 1) & 0x55555555;
@@ -24,29 +24,30 @@ unsigned int hweight32(unsigned int w)
 	return (res + (res >> 16)) & 0x000000FF;
 #endif
 }
-EXPORT_SYMBOL(hweight32);
+EXPORT_SYMBOL(__sw_hweight32);
 
-unsigned int hweight16(unsigned int w)
+unsigned int __sw_hweight16(unsigned int w)
 {
 	unsigned int res = w - ((w >> 1) & 0x5555);
 	res = (res & 0x3333) + ((res >> 2) & 0x3333);
 	res = (res + (res >> 4)) & 0x0F0F;
 	return (res + (res >> 8)) & 0x00FF;
 }
-EXPORT_SYMBOL(hweight16);
+EXPORT_SYMBOL(__sw_hweight16);
 
-unsigned int hweight8(unsigned int w)
+unsigned int __sw_hweight8(unsigned int w)
 {
 	unsigned int res = w - ((w >> 1) & 0x55);
 	res = (res & 0x33) + ((res >> 2) & 0x33);
 	return (res + (res >> 4)) & 0x0F;
 }
-EXPORT_SYMBOL(hweight8);
+EXPORT_SYMBOL(__sw_hweight8);
 
-unsigned long hweight64(__u64 w)
+unsigned long __sw_hweight64(__u64 w)
 {
 #if BITS_PER_LONG == 32
-	return hweight32((unsigned int)(w >> 32)) + hweight32((unsigned int)w);
+	return __sw_hweight32((unsigned int)(w >> 32)) +
+	       __sw_hweight32((unsigned int)w);
 #elif BITS_PER_LONG == 64
 #ifdef ARCH_HAS_FAST_MULTIPLIER
 	w -= (w >> 1) & 0x5555555555555555ul;
@@ -63,4 +64,4 @@ unsigned long hweight64(__u64 w)
 #endif
 #endif
 }
-EXPORT_SYMBOL(hweight64);
+EXPORT_SYMBOL(__sw_hweight64);
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index f9bdf26..cbcd654 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -245,3 +245,7 @@ quiet_cmd_lzo = LZO    $@
 cmd_lzo = (cat $(filter-out FORCE,$^) | \
 	lzop -9 && $(call size_append, $(filter-out FORCE,$^))) > $@ || \
 	(rm -f $@ ; false)
+
+# misc stuff
+# ---------------------------------------------------------------------------
+quote:="

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [GIT PULL] core/hweight changes for v2.6.35
  2010-05-17 21:21 [GIT PULL] core/hweight changes for v2.6.35 Ingo Molnar
@ 2010-05-17 21:46 ` Linus Torvalds
  2010-05-17 22:06   ` H. Peter Anvin
                     ` (2 more replies)
  2010-08-13 21:42 ` [GIT PULL] " Mike Frysinger
  1 sibling, 3 replies; 12+ messages in thread
From: Linus Torvalds @ 2010-05-17 21:46 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, H. Peter Anvin, Borislav Petkov, Peter Zijlstra,
	Thomas Gleixner, Andrew Morton



On Mon, 17 May 2010, Ingo Molnar wrote:
>
> +#ifdef CONFIG_64BIT
> +/* popcnt %rdi, %rax */
> +#define POPCNT ".byte 0xf3,0x48,0x0f,0xb8,0xc7"
> +#define REG_IN "D"
> +#define REG_OUT "a"
...
> +/*
> + * __sw_hweightXX are called from within the alternatives below
> + * and callee-clobbered registers need to be taken care of. See
> + * ARCH_HWEIGHT_CFLAGS in <arch/x86/Kconfig> for the respective
> + * compiler switches.
> + */
> +static inline unsigned int __arch_hweight32(unsigned int w)
> +{
> +	unsigned int res = 0;
> +
> +	asm (ALTERNATIVE("call __sw_hweight32", POPCNT, X86_FEATURE_POPCNT)
> +		     : "="REG_OUT (res)
> +		     : REG_IN (w));
> +
> +	return res;
> +}

I do not believe this is correct.

On x86-64, you are using a 64-bit instruction, but

	REG_IN (w)

does _not_ guarantee that the register is zero in the high bits.

Yes, yes, in practice it _probably_ is, because the register almost 
certainly got loaded with some kind of zero-extending mov instruction. But 
as far as I can tell, the code is buggy. You're telling gcc that you are 
using a 32-bit register, but you're actually counting bits in the full 64 
bits.

Am I missing something?

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GIT PULL] core/hweight changes for v2.6.35
  2010-05-17 21:46 ` Linus Torvalds
@ 2010-05-17 22:06   ` H. Peter Anvin
  2010-05-17 22:57   ` [tip:core/hweight] x86, hweight: Use a 32-bit popcnt for __arch_hweight32() tip-bot for H. Peter Anvin
  2010-05-17 23:04   ` [GIT PULL] UPDATED - core/hweight changes for v2.6.35 H. Peter Anvin
  2 siblings, 0 replies; 12+ messages in thread
From: H. Peter Anvin @ 2010-05-17 22:06 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Ingo Molnar, linux-kernel, Borislav Petkov, Peter Zijlstra,
	Thomas Gleixner, Andrew Morton

On 05/17/2010 02:46 PM, Linus Torvalds wrote:
> 
> 
> On Mon, 17 May 2010, Ingo Molnar wrote:
>>
>> +#ifdef CONFIG_64BIT
>> +/* popcnt %rdi, %rax */
>> +#define POPCNT ".byte 0xf3,0x48,0x0f,0xb8,0xc7"
>> +#define REG_IN "D"
>> +#define REG_OUT "a"
> ...
>> +/*
>> + * __sw_hweightXX are called from within the alternatives below
>> + * and callee-clobbered registers need to be taken care of. See
>> + * ARCH_HWEIGHT_CFLAGS in <arch/x86/Kconfig> for the respective
>> + * compiler switches.
>> + */
>> +static inline unsigned int __arch_hweight32(unsigned int w)
>> +{
>> +	unsigned int res = 0;
>> +
>> +	asm (ALTERNATIVE("call __sw_hweight32", POPCNT, X86_FEATURE_POPCNT)
>> +		     : "="REG_OUT (res)
>> +		     : REG_IN (w));
>> +
>> +	return res;
>> +}
> 
> I do not believe this is correct.
> 
> On x86-64, you are using a 64-bit instruction, but
> 
> 	REG_IN (w)
> 
> does _not_ guarantee that the register is zero in the high bits.
> 
> Yes, yes, in practice it _probably_ is, because the register almost 
> certainly got loaded with some kind of zero-extending mov instruction. But 
> as far as I can tell, the code is buggy. You're telling gcc that you are 
> using a 32-bit register, but you're actually counting bits in the full 64 
> bits.
> 
> Am I missing something?
> 

No, you're absolutely right; the input can be any value including
sign-extended.  The best fix for this is probably to use a 32-bit
instruction in this case -- I will check in such a patch and update the
pull branch.  If that's okay with you we'll send you a new pull request
in a bit.

The embarrassing part is I think I spotted this problem at one point,
but forgot about it in dealing with a conflict between this and another
patchset.

	-hpa


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip:core/hweight] x86, hweight: Use a 32-bit popcnt for __arch_hweight32()
  2010-05-17 21:46 ` Linus Torvalds
  2010-05-17 22:06   ` H. Peter Anvin
@ 2010-05-17 22:57   ` tip-bot for H. Peter Anvin
  2010-05-17 23:04   ` [GIT PULL] UPDATED - core/hweight changes for v2.6.35 H. Peter Anvin
  2 siblings, 0 replies; 12+ messages in thread
From: tip-bot for H. Peter Anvin @ 2010-05-17 22:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, torvalds, tglx, hpa, borislav.petkov

Commit-ID:  c59bd5688299cddb71183e156e7a3c1409b90df2
Gitweb:     http://git.kernel.org/tip/c59bd5688299cddb71183e156e7a3c1409b90df2
Author:     H. Peter Anvin <hpa@linux.intel.com>
AuthorDate: Mon, 17 May 2010 15:13:23 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Mon, 17 May 2010 15:17:16 -0700

x86, hweight: Use a 32-bit popcnt for __arch_hweight32()

Use a 32-bit popcnt instruction for __arch_hweight32(), even on
x86-64.  Even though the input register will *usually* be
zero-extended due to the standard operation of the hardware, it isn't
necessarily so if the input value was the result of truncating a
64-bit operation.

Note: the POPCNT32 variant used on x86-64 has a technically
unnecessary REX prefix to make it five bytes long, the same as a CALL
instruction, therefore avoiding an unnecessary NOP.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
Cc: Borislav Petkov <borislav.petkov@amd.com>
LKML-Reference: <alpine.LFD.2.00.1005171443060.4195@i5.linux-foundation.org>
---
 arch/x86/include/asm/arch_hweight.h |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h
index d1fc3c2..9686c3d 100644
--- a/arch/x86/include/asm/arch_hweight.h
+++ b/arch/x86/include/asm/arch_hweight.h
@@ -2,13 +2,15 @@
 #define _ASM_X86_HWEIGHT_H
 
 #ifdef CONFIG_64BIT
+/* popcnt %edi, %eax -- redundant REX prefix for alignment */
+#define POPCNT32 ".byte 0xf3,0x40,0x0f,0xb8,0xc7"
 /* popcnt %rdi, %rax */
-#define POPCNT ".byte 0xf3,0x48,0x0f,0xb8,0xc7"
+#define POPCNT64 ".byte 0xf3,0x48,0x0f,0xb8,0xc7"
 #define REG_IN "D"
 #define REG_OUT "a"
 #else
 /* popcnt %eax, %eax */
-#define POPCNT ".byte 0xf3,0x0f,0xb8,0xc0"
+#define POPCNT32 ".byte 0xf3,0x0f,0xb8,0xc0"
 #define REG_IN "a"
 #define REG_OUT "a"
 #endif
@@ -23,7 +25,7 @@ static inline unsigned int __arch_hweight32(unsigned int w)
 {
 	unsigned int res = 0;
 
-	asm (ALTERNATIVE("call __sw_hweight32", POPCNT, X86_FEATURE_POPCNT)
+	asm (ALTERNATIVE("call __sw_hweight32", POPCNT32, X86_FEATURE_POPCNT)
 		     : "="REG_OUT (res)
 		     : REG_IN (w));
 
@@ -48,7 +50,7 @@ static inline unsigned long __arch_hweight64(__u64 w)
 	return  __arch_hweight32((u32)w) +
 		__arch_hweight32((u32)(w >> 32));
 #else
-	asm (ALTERNATIVE("call __sw_hweight64", POPCNT, X86_FEATURE_POPCNT)
+	asm (ALTERNATIVE("call __sw_hweight64", POPCNT64, X86_FEATURE_POPCNT)
 		     : "="REG_OUT (res)
 		     : REG_IN (w));
 #endif /* CONFIG_X86_32 */

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [GIT PULL] UPDATED - core/hweight changes for v2.6.35
  2010-05-17 21:46 ` Linus Torvalds
  2010-05-17 22:06   ` H. Peter Anvin
  2010-05-17 22:57   ` [tip:core/hweight] x86, hweight: Use a 32-bit popcnt for __arch_hweight32() tip-bot for H. Peter Anvin
@ 2010-05-17 23:04   ` H. Peter Anvin
  2 siblings, 0 replies; 12+ messages in thread
From: H. Peter Anvin @ 2010-05-17 23:04 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, Ingo Molnar, Thomas Gleixner,
	Borislav Petkov, Peter Zijlstra, Andrew Morton

Hi Linus,

Here is the updated core/hweight tree.

The following changes since commit 0fdf86754f70e813845af4abaa805165ce57a0bb:
  Linus Torvalds (1):
        Merge branch 'urgent' of git://git.kernel.org/.../brodo/pcmcia-2.6

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core-hweight-for-linus

Borislav Petkov (2):
      x86: Add optimized popcnt variants
      arch, hweight: Fix compilation errors

H. Peter Anvin (1):
      x86, hweight: Use a 32-bit popcnt for __arch_hweight32()

Peter Zijlstra (1):
      bitops: Optimize hweight() by making use of compile-time evaluation

 arch/alpha/include/asm/bitops.h            |   18 +++++----
 arch/ia64/include/asm/bitops.h             |   11 +++--
 arch/sparc/include/asm/bitops_64.h         |   11 +++--
 arch/x86/Kconfig                           |    5 ++
 arch/x86/include/asm/alternative.h         |    9 +++-
 arch/x86/include/asm/arch_hweight.h        |   61 ++++++++++++++++++++++++++++
 arch/x86/include/asm/bitops.h              |    4 +-
 include/asm-generic/bitops/arch_hweight.h  |   25 +++++++++++
 include/asm-generic/bitops/const_hweight.h |   42 +++++++++++++++++++
 include/asm-generic/bitops/hweight.h       |    8 +---
 include/linux/bitops.h                     |   30 ++-----------
 lib/Makefile                               |    3 +
 lib/hweight.c                              |   19 +++++----
 scripts/Makefile.lib                       |    4 ++
 14 files changed, 188 insertions(+), 62 deletions(-)
 create mode 100644 arch/x86/include/asm/arch_hweight.h
 create mode 100644 include/asm-generic/bitops/arch_hweight.h
 create mode 100644 include/asm-generic/bitops/const_hweight.h

diff --git a/arch/alpha/include/asm/bitops.h b/arch/alpha/include/asm/bitops.h
index 15f3ae2..296da1d 100644
--- a/arch/alpha/include/asm/bitops.h
+++ b/arch/alpha/include/asm/bitops.h
@@ -405,29 +405,31 @@ static inline int fls(int x)
 
 #if defined(CONFIG_ALPHA_EV6) && defined(CONFIG_ALPHA_EV67)
 /* Whee.  EV67 can calculate it directly.  */
-static inline unsigned long hweight64(unsigned long w)
+static inline unsigned long __arch_hweight64(unsigned long w)
 {
 	return __kernel_ctpop(w);
 }
 
-static inline unsigned int hweight32(unsigned int w)
+static inline unsigned int __arch_weight32(unsigned int w)
 {
-	return hweight64(w);
+	return __arch_hweight64(w);
 }
 
-static inline unsigned int hweight16(unsigned int w)
+static inline unsigned int __arch_hweight16(unsigned int w)
 {
-	return hweight64(w & 0xffff);
+	return __arch_hweight64(w & 0xffff);
 }
 
-static inline unsigned int hweight8(unsigned int w)
+static inline unsigned int __arch_hweight8(unsigned int w)
 {
-	return hweight64(w & 0xff);
+	return __arch_hweight64(w & 0xff);
 }
 #else
-#include <asm-generic/bitops/hweight.h>
+#include <asm-generic/bitops/arch_hweight.h>
 #endif
 
+#include <asm-generic/bitops/const_hweight.h>
+
 #endif /* __KERNEL__ */
 
 #include <asm-generic/bitops/find.h>
diff --git a/arch/ia64/include/asm/bitops.h b/arch/ia64/include/asm/bitops.h
index 6ebc229..9da3df6 100644
--- a/arch/ia64/include/asm/bitops.h
+++ b/arch/ia64/include/asm/bitops.h
@@ -437,17 +437,18 @@ __fls (unsigned long x)
  * hweightN: returns the hamming weight (i.e. the number
  * of bits set) of a N-bit word
  */
-static __inline__ unsigned long
-hweight64 (unsigned long x)
+static __inline__ unsigned long __arch_hweight64(unsigned long x)
 {
 	unsigned long result;
 	result = ia64_popcnt(x);
 	return result;
 }
 
-#define hweight32(x)	(unsigned int) hweight64((x) & 0xfffffffful)
-#define hweight16(x)	(unsigned int) hweight64((x) & 0xfffful)
-#define hweight8(x)	(unsigned int) hweight64((x) & 0xfful)
+#define __arch_hweight32(x) ((unsigned int) __arch_hweight64((x) & 0xfffffffful))
+#define __arch_hweight16(x) ((unsigned int) __arch_hweight64((x) & 0xfffful))
+#define __arch_hweight8(x)  ((unsigned int) __arch_hweight64((x) & 0xfful))
+
+#include <asm-generic/bitops/const_hweight.h>
 
 #endif /* __KERNEL__ */
 
diff --git a/arch/sparc/include/asm/bitops_64.h b/arch/sparc/include/asm/bitops_64.h
index e72ac9c..766121a 100644
--- a/arch/sparc/include/asm/bitops_64.h
+++ b/arch/sparc/include/asm/bitops_64.h
@@ -44,7 +44,7 @@ extern void change_bit(unsigned long nr, volatile unsigned long *addr);
 
 #ifdef ULTRA_HAS_POPULATION_COUNT
 
-static inline unsigned int hweight64(unsigned long w)
+static inline unsigned int __arch_hweight64(unsigned long w)
 {
 	unsigned int res;
 
@@ -52,7 +52,7 @@ static inline unsigned int hweight64(unsigned long w)
 	return res;
 }
 
-static inline unsigned int hweight32(unsigned int w)
+static inline unsigned int __arch_hweight32(unsigned int w)
 {
 	unsigned int res;
 
@@ -60,7 +60,7 @@ static inline unsigned int hweight32(unsigned int w)
 	return res;
 }
 
-static inline unsigned int hweight16(unsigned int w)
+static inline unsigned int __arch_hweight16(unsigned int w)
 {
 	unsigned int res;
 
@@ -68,7 +68,7 @@ static inline unsigned int hweight16(unsigned int w)
 	return res;
 }
 
-static inline unsigned int hweight8(unsigned int w)
+static inline unsigned int __arch_hweight8(unsigned int w)
 {
 	unsigned int res;
 
@@ -78,9 +78,10 @@ static inline unsigned int hweight8(unsigned int w)
 
 #else
 
-#include <asm-generic/bitops/hweight.h>
+#include <asm-generic/bitops/arch_hweight.h>
 
 #endif
+#include <asm-generic/bitops/const_hweight.h>
 #include <asm-generic/bitops/lock.h>
 #endif /* __KERNEL__ */
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0eacb1f..89d8c54 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -238,6 +238,11 @@ config X86_32_LAZY_GS
 	def_bool y
 	depends on X86_32 && !CC_STACKPROTECTOR
 
+config ARCH_HWEIGHT_CFLAGS
+	string
+	default "-fcall-saved-ecx -fcall-saved-edx" if X86_32
+	default "-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fcall-saved-r11" if X86_64
+
 config KTIME_SCALAR
 	def_bool X86_32
 source "init/Kconfig"
diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index b09ec55..67dae51 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -39,9 +39,6 @@
 #define LOCK_PREFIX ""
 #endif
 
-/* This must be included *after* the definition of LOCK_PREFIX */
-#include <asm/cpufeature.h>
-
 struct alt_instr {
 	u8 *instr;		/* original instruction */
 	u8 *replacement;
@@ -96,6 +93,12 @@ static inline int alternatives_text_reserved(void *start, void *end)
       ".previous"
 
 /*
+ * This must be included *after* the definition of ALTERNATIVE due to
+ * <asm/arch_hweight.h>
+ */
+#include <asm/cpufeature.h>
+
+/*
  * Alternative instructions for different CPU types or capabilities.
  *
  * This allows to use optimized instructions even on generic binary
diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h
new file mode 100644
index 0000000..9686c3d
--- /dev/null
+++ b/arch/x86/include/asm/arch_hweight.h
@@ -0,0 +1,61 @@
+#ifndef _ASM_X86_HWEIGHT_H
+#define _ASM_X86_HWEIGHT_H
+
+#ifdef CONFIG_64BIT
+/* popcnt %edi, %eax -- redundant REX prefix for alignment */
+#define POPCNT32 ".byte 0xf3,0x40,0x0f,0xb8,0xc7"
+/* popcnt %rdi, %rax */
+#define POPCNT64 ".byte 0xf3,0x48,0x0f,0xb8,0xc7"
+#define REG_IN "D"
+#define REG_OUT "a"
+#else
+/* popcnt %eax, %eax */
+#define POPCNT32 ".byte 0xf3,0x0f,0xb8,0xc0"
+#define REG_IN "a"
+#define REG_OUT "a"
+#endif
+
+/*
+ * __sw_hweightXX are called from within the alternatives below
+ * and callee-clobbered registers need to be taken care of. See
+ * ARCH_HWEIGHT_CFLAGS in <arch/x86/Kconfig> for the respective
+ * compiler switches.
+ */
+static inline unsigned int __arch_hweight32(unsigned int w)
+{
+	unsigned int res = 0;
+
+	asm (ALTERNATIVE("call __sw_hweight32", POPCNT32, X86_FEATURE_POPCNT)
+		     : "="REG_OUT (res)
+		     : REG_IN (w));
+
+	return res;
+}
+
+static inline unsigned int __arch_hweight16(unsigned int w)
+{
+	return __arch_hweight32(w & 0xffff);
+}
+
+static inline unsigned int __arch_hweight8(unsigned int w)
+{
+	return __arch_hweight32(w & 0xff);
+}
+
+static inline unsigned long __arch_hweight64(__u64 w)
+{
+	unsigned long res = 0;
+
+#ifdef CONFIG_X86_32
+	return  __arch_hweight32((u32)w) +
+		__arch_hweight32((u32)(w >> 32));
+#else
+	asm (ALTERNATIVE("call __sw_hweight64", POPCNT64, X86_FEATURE_POPCNT)
+		     : "="REG_OUT (res)
+		     : REG_IN (w));
+#endif /* CONFIG_X86_32 */
+
+	return res;
+}
+
+#endif
diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index 02b47a6..545776e 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -444,7 +444,9 @@ static inline int fls(int x)
 
 #define ARCH_HAS_FAST_MULTIPLIER 1
 
-#include <asm-generic/bitops/hweight.h>
+#include <asm/arch_hweight.h>
+
+#include <asm-generic/bitops/const_hweight.h>
 
 #endif /* __KERNEL__ */
 
diff --git a/include/asm-generic/bitops/arch_hweight.h b/include/asm-generic/bitops/arch_hweight.h
new file mode 100644
index 0000000..6a211f4
--- /dev/null
+++ b/include/asm-generic/bitops/arch_hweight.h
@@ -0,0 +1,25 @@
+#ifndef _ASM_GENERIC_BITOPS_ARCH_HWEIGHT_H_
+#define _ASM_GENERIC_BITOPS_ARCH_HWEIGHT_H_
+
+#include <asm/types.h>
+
+static inline unsigned int __arch_hweight32(unsigned int w)
+{
+	return __sw_hweight32(w);
+}
+
+static inline unsigned int __arch_hweight16(unsigned int w)
+{
+	return __sw_hweight16(w);
+}
+
+static inline unsigned int __arch_hweight8(unsigned int w)
+{
+	return __sw_hweight8(w);
+}
+
+static inline unsigned long __arch_hweight64(__u64 w)
+{
+	return __sw_hweight64(w);
+}
+#endif /* _ASM_GENERIC_BITOPS_HWEIGHT_H_ */
diff --git a/include/asm-generic/bitops/const_hweight.h b/include/asm-generic/bitops/const_hweight.h
new file mode 100644
index 0000000..fa2a50b
--- /dev/null
+++ b/include/asm-generic/bitops/const_hweight.h
@@ -0,0 +1,42 @@
+#ifndef _ASM_GENERIC_BITOPS_CONST_HWEIGHT_H_
+#define _ASM_GENERIC_BITOPS_CONST_HWEIGHT_H_
+
+/*
+ * Compile time versions of __arch_hweightN()
+ */
+#define __const_hweight8(w)		\
+      (	(!!((w) & (1ULL << 0))) +	\
+	(!!((w) & (1ULL << 1))) +	\
+	(!!((w) & (1ULL << 2))) +	\
+	(!!((w) & (1ULL << 3))) +	\
+	(!!((w) & (1ULL << 4))) +	\
+	(!!((w) & (1ULL << 5))) +	\
+	(!!((w) & (1ULL << 6))) +	\
+	(!!((w) & (1ULL << 7)))	)
+
+#define __const_hweight16(w) (__const_hweight8(w)  + __const_hweight8((w)  >> 8 ))
+#define __const_hweight32(w) (__const_hweight16(w) + __const_hweight16((w) >> 16))
+#define __const_hweight64(w) (__const_hweight32(w) + __const_hweight32((w) >> 32))
+
+/*
+ * Generic interface.
+ */
+#define hweight8(w)  (__builtin_constant_p(w) ? __const_hweight8(w)  : __arch_hweight8(w))
+#define hweight16(w) (__builtin_constant_p(w) ? __const_hweight16(w) : __arch_hweight16(w))
+#define hweight32(w) (__builtin_constant_p(w) ? __const_hweight32(w) : __arch_hweight32(w))
+#define hweight64(w) (__builtin_constant_p(w) ? __const_hweight64(w) : __arch_hweight64(w))
+
+/*
+ * Interface for known constant arguments
+ */
+#define HWEIGHT8(w)  (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight8(w))
+#define HWEIGHT16(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight16(w))
+#define HWEIGHT32(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight32(w))
+#define HWEIGHT64(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight64(w))
+
+/*
+ * Type invariant interface to the compile time constant hweight functions.
+ */
+#define HWEIGHT(w)   HWEIGHT64((u64)w)
+
+#endif /* _ASM_GENERIC_BITOPS_CONST_HWEIGHT_H_ */
diff --git a/include/asm-generic/bitops/hweight.h b/include/asm-generic/bitops/hweight.h
index fbbc383..a94d651 100644
--- a/include/asm-generic/bitops/hweight.h
+++ b/include/asm-generic/bitops/hweight.h
@@ -1,11 +1,7 @@
 #ifndef _ASM_GENERIC_BITOPS_HWEIGHT_H_
 #define _ASM_GENERIC_BITOPS_HWEIGHT_H_
 
-#include <asm/types.h>
-
-extern unsigned int hweight32(unsigned int w);
-extern unsigned int hweight16(unsigned int w);
-extern unsigned int hweight8(unsigned int w);
-extern unsigned long hweight64(__u64 w);
+#include <asm-generic/bitops/arch_hweight.h>
+#include <asm-generic/bitops/const_hweight.h>
 
 #endif /* _ASM_GENERIC_BITOPS_HWEIGHT_H_ */
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index b793898..26caa60 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -10,6 +10,11 @@
 #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 #endif
 
+extern unsigned int __sw_hweight8(unsigned int w);
+extern unsigned int __sw_hweight16(unsigned int w);
+extern unsigned int __sw_hweight32(unsigned int w);
+extern unsigned long __sw_hweight64(__u64 w);
+
 /*
  * Include this here because some architectures need generic_ffs/fls in
  * scope
@@ -47,31 +52,6 @@ static inline unsigned long hweight_long(unsigned long w)
 	return sizeof(w) == 4 ? hweight32(w) : hweight64(w);
 }
 
-/*
- * Clearly slow versions of the hweightN() functions, their benefit is
- * of course compile time evaluation of constant arguments.
- */
-#define HWEIGHT8(w)					\
-      (	BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) +	\
-	(!!((w) & (1ULL << 0))) +			\
-	(!!((w) & (1ULL << 1))) +			\
-	(!!((w) & (1ULL << 2))) +			\
-	(!!((w) & (1ULL << 3))) +			\
-	(!!((w) & (1ULL << 4))) +			\
-	(!!((w) & (1ULL << 5))) +			\
-	(!!((w) & (1ULL << 6))) +			\
-	(!!((w) & (1ULL << 7)))	)
-
-#define HWEIGHT16(w) (HWEIGHT8(w)  + HWEIGHT8((w) >> 8))
-#define HWEIGHT32(w) (HWEIGHT16(w) + HWEIGHT16((w) >> 16))
-#define HWEIGHT64(w) (HWEIGHT32(w) + HWEIGHT32((w) >> 32))
-
-/*
- * Type invariant version that simply casts things to the
- * largest type.
- */
-#define HWEIGHT(w)   HWEIGHT64((u64)(w))
-
 /**
  * rol32 - rotate a 32-bit value left
  * @word: value to rotate
diff --git a/lib/Makefile b/lib/Makefile
index 2e152ae..abe63a8 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -39,7 +39,10 @@ lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
 lib-$(CONFIG_GENERIC_FIND_FIRST_BIT) += find_next_bit.o
 lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find_next_bit.o
 obj-$(CONFIG_GENERIC_FIND_LAST_BIT) += find_last_bit.o
+
+CFLAGS_hweight.o = $(subst $(quote),,$(CONFIG_ARCH_HWEIGHT_CFLAGS))
 obj-$(CONFIG_GENERIC_HWEIGHT) += hweight.o
+
 obj-$(CONFIG_LOCK_KERNEL) += kernel_lock.o
 obj-$(CONFIG_BTREE) += btree.o
 obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o
diff --git a/lib/hweight.c b/lib/hweight.c
index 63ee4eb..3c79d50 100644
--- a/lib/hweight.c
+++ b/lib/hweight.c
@@ -9,7 +9,7 @@
  * The Hamming Weight of a number is the total number of bits set in it.
  */
 
-unsigned int hweight32(unsigned int w)
+unsigned int __sw_hweight32(unsigned int w)
 {
 #ifdef ARCH_HAS_FAST_MULTIPLIER
 	w -= (w >> 1) & 0x55555555;
@@ -24,29 +24,30 @@ unsigned int hweight32(unsigned int w)
 	return (res + (res >> 16)) & 0x000000FF;
 #endif
 }
-EXPORT_SYMBOL(hweight32);
+EXPORT_SYMBOL(__sw_hweight32);
 
-unsigned int hweight16(unsigned int w)
+unsigned int __sw_hweight16(unsigned int w)
 {
 	unsigned int res = w - ((w >> 1) & 0x5555);
 	res = (res & 0x3333) + ((res >> 2) & 0x3333);
 	res = (res + (res >> 4)) & 0x0F0F;
 	return (res + (res >> 8)) & 0x00FF;
 }
-EXPORT_SYMBOL(hweight16);
+EXPORT_SYMBOL(__sw_hweight16);
 
-unsigned int hweight8(unsigned int w)
+unsigned int __sw_hweight8(unsigned int w)
 {
 	unsigned int res = w - ((w >> 1) & 0x55);
 	res = (res & 0x33) + ((res >> 2) & 0x33);
 	return (res + (res >> 4)) & 0x0F;
 }
-EXPORT_SYMBOL(hweight8);
+EXPORT_SYMBOL(__sw_hweight8);
 
-unsigned long hweight64(__u64 w)
+unsigned long __sw_hweight64(__u64 w)
 {
 #if BITS_PER_LONG == 32
-	return hweight32((unsigned int)(w >> 32)) + hweight32((unsigned int)w);
+	return __sw_hweight32((unsigned int)(w >> 32)) +
+	       __sw_hweight32((unsigned int)w);
 #elif BITS_PER_LONG == 64
 #ifdef ARCH_HAS_FAST_MULTIPLIER
 	w -= (w >> 1) & 0x5555555555555555ul;
@@ -63,4 +64,4 @@ unsigned long hweight64(__u64 w)
 #endif
 #endif
 }
-EXPORT_SYMBOL(hweight64);
+EXPORT_SYMBOL(__sw_hweight64);
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index f9bdf26..cbcd654 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -245,3 +245,7 @@ quiet_cmd_lzo = LZO    $@
 cmd_lzo = (cat $(filter-out FORCE,$^) | \
 	lzop -9 && $(call size_append, $(filter-out FORCE,$^))) > $@ || \
 	(rm -f $@ ; false)
+
+# misc stuff
+# ---------------------------------------------------------------------------
+quote:="

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [GIT PULL] core/hweight changes for v2.6.35
  2010-05-17 21:21 [GIT PULL] core/hweight changes for v2.6.35 Ingo Molnar
  2010-05-17 21:46 ` Linus Torvalds
@ 2010-08-13 21:42 ` Mike Frysinger
  2010-08-13 21:56   ` H. Peter Anvin
  2010-08-14  0:32   ` [PATCH] arch/tile: Rename the hweight() implementations to __arch_hweight() Chris Metcalf
  1 sibling, 2 replies; 12+ messages in thread
From: Mike Frysinger @ 2010-08-13 21:42 UTC (permalink / raw)
  To: Borislav Petkov, Chris Metcalf
  Cc: Linus Torvalds, linux-kernel, H. Peter Anvin, Peter Zijlstra,
	Thomas Gleixner, Andrew Morton, Ingo Molnar

On Mon, May 17, 2010 at 17:21, Ingo Molnar wrote:
>  arch/alpha/include/asm/bitops.h            |   18 +++++----
>  arch/ia64/include/asm/bitops.h             |   11 +++--
>  arch/sparc/include/asm/bitops_64.h         |   11 +++--
>  arch/x86/include/asm/bitops.h              |    4 +-
>  include/asm-generic/bitops/arch_hweight.h  |   25 ++++++++++++
>  include/asm-generic/bitops/const_hweight.h |   42 ++++++++++++++++++++
>  include/asm-generic/bitops/hweight.h       |    8 +---

did this miss Blackfin because the original patch was against the
2.6.34 tree ?  just wondering why it now build fails ...

doing a simple grep shows that the new "tile" arch may also be broken
as it uses "hweight32" ...

considering __sw_hweightX only exist when the generic hweight is in
play, wouldnt it make sense to have
include/asm-generic/bitops/arch_hweight.h not always define things ?
then most arches can simply pull in
include/asm-generic/bitops/hweight.h without having to worry about the
random inner details of hweight cruft.

diff --git a/include/asm-generic/bitops/arch_hweight.h
b/include/asm-generic/bitops/arch_hweight.h
index 6a211f4..3a62e38 100644
--- a/include/asm-generic/bitops/arch_hweight.h
+++ b/include/asm-generic/bitops/arch_hweight.h
@@ -1,8 +1,15 @@
 #ifndef _ASM_GENERIC_BITOPS_ARCH_HWEIGHT_H_
 #define _ASM_GENERIC_BITOPS_ARCH_HWEIGHT_H_

+#ifdef CONFIG_GENERIC_HWEIGHT
+
 #include <asm/types.h>

+extern unsigned int __sw_hweight8(unsigned int w);
+extern unsigned int __sw_hweight16(unsigned int w);
+extern unsigned int __sw_hweight32(unsigned int w);
+extern unsigned long __sw_hweight64(__u64 w);
+
 static inline unsigned int __arch_hweight32(unsigned int w)
 {
    return __sw_hweight32(w);
@@ -22,4 +29,7 @@ static inline unsigned long __arch_hweight64(__u64 w)
 {
    return __sw_hweight64(w);
 }
+
+#endif /* CONFIG_GENERIC_HWEIGHT */
+
 #endif /* _ASM_GENERIC_BITOPS_HWEIGHT_H_ */
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index fc68053..2e58012 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -10,11 +10,6 @@
 #define BITS_TO_LONGS(nr)  DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 #endif

-extern unsigned int __sw_hweight8(unsigned int w);
-extern unsigned int __sw_hweight16(unsigned int w);
-extern unsigned int __sw_hweight32(unsigned int w);
-extern unsigned long __sw_hweight64(__u64 w);
-
 /*
  * Include this here because some architectures need generic_ffs/fls in
  * scope
-mike

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [GIT PULL] core/hweight changes for v2.6.35
  2010-08-13 21:42 ` [GIT PULL] " Mike Frysinger
@ 2010-08-13 21:56   ` H. Peter Anvin
  2010-08-13 21:59     ` Mike Frysinger
  2010-08-14  0:32   ` [PATCH] arch/tile: Rename the hweight() implementations to __arch_hweight() Chris Metcalf
  1 sibling, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2010-08-13 21:56 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: Borislav Petkov, Chris Metcalf, Linus Torvalds, linux-kernel,
	Peter Zijlstra, Thomas Gleixner, Andrew Morton, Ingo Molnar

On 08/13/2010 02:42 PM, Mike Frysinger wrote:
> On Mon, May 17, 2010 at 17:21, Ingo Molnar wrote:
>>  arch/alpha/include/asm/bitops.h            |   18 +++++----
>>  arch/ia64/include/asm/bitops.h             |   11 +++--
>>  arch/sparc/include/asm/bitops_64.h         |   11 +++--
>>  arch/x86/include/asm/bitops.h              |    4 +-
>>  include/asm-generic/bitops/arch_hweight.h  |   25 ++++++++++++
>>  include/asm-generic/bitops/const_hweight.h |   42 ++++++++++++++++++++
>>  include/asm-generic/bitops/hweight.h       |    8 +---
> 
> did this miss Blackfin because the original patch was against the
> 2.6.34 tree ?  just wondering why it now build fails ...
> 
> doing a simple grep shows that the new "tile" arch may also be broken
> as it uses "hweight32" ...
> 
> considering __sw_hweightX only exist when the generic hweight is in
> play, wouldnt it make sense to have
> include/asm-generic/bitops/arch_hweight.h not always define things ?
> then most arches can simply pull in
> include/asm-generic/bitops/hweight.h without having to worry about the
> random inner details of hweight cruft.
> 

__sw_hweightX can exist even when generic hweight isn't in use per se,
because the arch implementation can wrapper the software implementation.
 This is the case on x86, for example -- most x86 CPUs don't have popcnt
yet, so on those the x86 implementation end up calling the
__sw_hweight*() implementations.

	-hpa

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GIT PULL] core/hweight changes for v2.6.35
  2010-08-13 21:56   ` H. Peter Anvin
@ 2010-08-13 21:59     ` Mike Frysinger
  2010-08-14  0:11       ` H. Peter Anvin
  0 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2010-08-13 21:59 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Chris Metcalf, Linus Torvalds, linux-kernel,
	Peter Zijlstra, Thomas Gleixner, Andrew Morton, Ingo Molnar

On Fri, Aug 13, 2010 at 17:56, H. Peter Anvin wrote:
> On 08/13/2010 02:42 PM, Mike Frysinger wrote:
>> On Mon, May 17, 2010 at 17:21, Ingo Molnar wrote:
>>>  arch/alpha/include/asm/bitops.h            |   18 +++++----
>>>  arch/ia64/include/asm/bitops.h             |   11 +++--
>>>  arch/sparc/include/asm/bitops_64.h         |   11 +++--
>>>  arch/x86/include/asm/bitops.h              |    4 +-
>>>  include/asm-generic/bitops/arch_hweight.h  |   25 ++++++++++++
>>>  include/asm-generic/bitops/const_hweight.h |   42 ++++++++++++++++++++
>>>  include/asm-generic/bitops/hweight.h       |    8 +---
>>
>> did this miss Blackfin because the original patch was against the
>> 2.6.34 tree ?  just wondering why it now build fails ...
>>
>> doing a simple grep shows that the new "tile" arch may also be broken
>> as it uses "hweight32" ...
>>
>> considering __sw_hweightX only exist when the generic hweight is in
>> play, wouldnt it make sense to have
>> include/asm-generic/bitops/arch_hweight.h not always define things ?
>> then most arches can simply pull in
>> include/asm-generic/bitops/hweight.h without having to worry about the
>> random inner details of hweight cruft.
>>
>
> __sw_hweightX can exist even when generic hweight isn't in use per se,
> because the arch implementation can wrapper the software implementation.
>  This is the case on x86, for example -- most x86 CPUs don't have popcnt
> yet, so on those the x86 implementation end up calling the
> __sw_hweight*() implementations.

but those targets still define CONFIG_GENERIC_HWEIGHT right ?  so at
the least, we should be wrapping the prototypes in linux/bitops.h with
that ...
-mike

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GIT PULL] core/hweight changes for v2.6.35
  2010-08-13 21:59     ` Mike Frysinger
@ 2010-08-14  0:11       ` H. Peter Anvin
  2010-08-14 13:44         ` Borislav Petkov
  0 siblings, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2010-08-14  0:11 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: Borislav Petkov, Chris Metcalf, Linus Torvalds, linux-kernel,
	Peter Zijlstra, Thomas Gleixner, Andrew Morton, Ingo Molnar

On 08/13/2010 02:59 PM, Mike Frysinger wrote:
> 
> but those targets still define CONFIG_GENERIC_HWEIGHT right ?  so at
> the least, we should be wrapping the prototypes in linux/bitops.h with
> that ...
> 

Yes, I guess they do.

	-hpa

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] arch/tile: Rename the hweight() implementations to __arch_hweight().
  2010-08-13 21:42 ` [GIT PULL] " Mike Frysinger
  2010-08-13 21:56   ` H. Peter Anvin
@ 2010-08-14  0:32   ` Chris Metcalf
  2010-08-14  1:02     ` Mike Frysinger
  1 sibling, 1 reply; 12+ messages in thread
From: Chris Metcalf @ 2010-08-14  0:32 UTC (permalink / raw)
  To: Borislav Petkov, Mike Frysinger
  Cc: Linus Torvalds, linux-kernel, H. Peter Anvin, Peter Zijlstra,
	Thomas Gleixner, Andrew Morton, Ingo Molnar

See commit 1527bc8b928dd1399c3d3467dd47d9ede210978a.

Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---

Mike, thanks for the pointer to the hweight issue.  Since we are using
__builtin_popcount() and the compiler can do constant propagation, etc.,
with it at compile-time, this is less critical, but it seems like the
easiest way to get the definitions of the HWEIGHT macros.

 arch/tile/include/asm/bitops.h |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/tile/include/asm/bitops.h b/arch/tile/include/asm/bitops.h
index 84600f3..6832b4b 100644
--- a/arch/tile/include/asm/bitops.h
+++ b/arch/tile/include/asm/bitops.h
@@ -98,26 +98,27 @@ static inline int fls64(__u64 w)
 	return (sizeof(__u64) * 8) - __builtin_clzll(w);
 }
 
-static inline unsigned int hweight32(unsigned int w)
+static inline unsigned int __arch_hweight32(unsigned int w)
 {
 	return __builtin_popcount(w);
 }
 
-static inline unsigned int hweight16(unsigned int w)
+static inline unsigned int __arch_hweight16(unsigned int w)
 {
 	return __builtin_popcount(w & 0xffff);
 }
 
-static inline unsigned int hweight8(unsigned int w)
+static inline unsigned int __arch_hweight8(unsigned int w)
 {
 	return __builtin_popcount(w & 0xff);
 }
 
-static inline unsigned long hweight64(__u64 w)
+static inline unsigned long __arch_hweight64(__u64 w)
 {
 	return __builtin_popcountll(w);
 }
 
+#include <asm-generic/bitops/const_hweight.h>
 #include <asm-generic/bitops/lock.h>
 #include <asm-generic/bitops/sched.h>
 #include <asm-generic/bitops/ext2-non-atomic.h>
-- 
1.6.5.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] arch/tile: Rename the hweight() implementations to __arch_hweight().
  2010-08-14  0:32   ` [PATCH] arch/tile: Rename the hweight() implementations to __arch_hweight() Chris Metcalf
@ 2010-08-14  1:02     ` Mike Frysinger
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Frysinger @ 2010-08-14  1:02 UTC (permalink / raw)
  To: Chris Metcalf
  Cc: Borislav Petkov, Linus Torvalds, linux-kernel, H. Peter Anvin,
	Peter Zijlstra, Thomas Gleixner, Andrew Morton, Ingo Molnar

On Fri, Aug 13, 2010 at 20:32, Chris Metcalf wrote:
> Mike, thanks for the pointer to the hweight issue.  Since we are using
> __builtin_popcount() and the compiler can do constant propagation, etc.,
> with it at compile-time, this is less critical, but it seems like the
> easiest way to get the definitions of the HWEIGHT macros.

interesting ... i wasnt aware of the popcount builtin.  the Blackfin
ISA has a "ONES" insn which does this.  guess we should implement the
popcount builtin too.
-mike

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GIT PULL] core/hweight changes for v2.6.35
  2010-08-14  0:11       ` H. Peter Anvin
@ 2010-08-14 13:44         ` Borislav Petkov
  0 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2010-08-14 13:44 UTC (permalink / raw)
  To: H. Peter Anvin, Mike Frysinger
  Cc: Chris Metcalf, Linus Torvalds, linux-kernel, Peter Zijlstra,
	Thomas Gleixner, Andrew Morton, Ingo Molnar

From: "H. Peter Anvin" <hpa@zytor.com>
Date: Fri, Aug 13, 2010 at 05:11:14PM -0700

> On 08/13/2010 02:59 PM, Mike Frysinger wrote:
> > 
> > but those targets still define CONFIG_GENERIC_HWEIGHT right ?  so at
> > the least, we should be wrapping the prototypes in linux/bitops.h with
> > that ...
> > 
> 
> Yes, I guess they do.

Actually, Mike's issue is valid, if I understand it correctly. From
looking at <arch/blackfin/include/asm/bitops.h>, Blackfin doesn't need
the generic software variants __sw_hweightXX at all, right?

In that case, <include/linux/bitops.h> is pulling in needlessly
unresolved symbols for arches which have their hw versions of
hweight and don't need sw variants. However, if you want to include
<asm-generic/bitops/hweight.h> and benefit from the compile-time
evaluation of the const hweight args, you're going to need to rename
the hweightXX() functions in <arch/blackfin/include/asm/bitops.h> to
__arch_hweightXX but that shouldn't be a problem I guess :).

Let me give your patch from couple of mails ago a quick spin...

-- 
Regards/Gruss,
    Boris.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2010-08-14 13:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-17 21:21 [GIT PULL] core/hweight changes for v2.6.35 Ingo Molnar
2010-05-17 21:46 ` Linus Torvalds
2010-05-17 22:06   ` H. Peter Anvin
2010-05-17 22:57   ` [tip:core/hweight] x86, hweight: Use a 32-bit popcnt for __arch_hweight32() tip-bot for H. Peter Anvin
2010-05-17 23:04   ` [GIT PULL] UPDATED - core/hweight changes for v2.6.35 H. Peter Anvin
2010-08-13 21:42 ` [GIT PULL] " Mike Frysinger
2010-08-13 21:56   ` H. Peter Anvin
2010-08-13 21:59     ` Mike Frysinger
2010-08-14  0:11       ` H. Peter Anvin
2010-08-14 13:44         ` Borislav Petkov
2010-08-14  0:32   ` [PATCH] arch/tile: Rename the hweight() implementations to __arch_hweight() Chris Metcalf
2010-08-14  1:02     ` Mike Frysinger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.