From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Gerst Subject: Re: [PATCH 2/5] bitops: compile time optimization for hweight_long(CONSTANT) Date: Thu, 4 Feb 2010 10:39:28 -0500 Message-ID: <73c1f2161002040739t65507f6aicec4fcb325552a18@mail.gmail.com> References: <20100130094515.475881280@intel.com> <20100201124825.cc024f2a.akpm@linux-foundation.org> <20100203133951.GA24357@localhost> <20100203070825.e36b3932.akpm@linux-foundation.org> <1265210157.24455.646.camel@laptop> <20100203074251.e2caa3f3.akpm@linux-foundation.org> <20100203181425.GB1367@aftab> <1265222875.24455.1020.camel@laptop> <4B69D362.10608@zytor.com> <20100204151050.GC32711@aftab> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "H. Peter Anvin" , Peter Zijlstra , Andrew Morton , Wu Fengguang , LKML , Jamie Lokier , Roland Dreier , Al Viro , "linux-fsdevel@vger.kernel.org" , Ingo Molnar To: Borislav Petkov Return-path: Received: from fg-out-1718.google.com ([72.14.220.156]:43330 "EHLO fg-out-1718.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932278Ab0BDPjb convert rfc822-to-8bit (ORCPT ); Thu, 4 Feb 2010 10:39:31 -0500 In-Reply-To: <20100204151050.GC32711@aftab> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 2010/2/4 Borislav Petkov : > On Wed, Feb 03, 2010 at 11:49:54AM -0800, H. Peter Anvin wrote: >> On 02/03/2010 10:47 AM, Peter Zijlstra wrote: >> > On Wed, 2010-02-03 at 19:14 +0100, Borislav Petkov wrote: >> > >> >> alternative("call hweightXX", "popcnt", X86_FEATURE_POPCNT) >> > >> > Make sure to apply a 0xff bitmask to the popcnt r16 call for hweig= ht8(), >> > and hweight64() needs a bit of magic for 32bit, but yes, something= like >> > that ought to work nicely. >> > >> >> Arguably the "best" option is to have the alternative being a jump t= o an >> out-of-line stub which does the necessary parameter marshalling befo= re >> calling a stub. =C2=A0This technique is already used in a few other = places. > > Ok, here's a first alpha prototype and completely untested. The asm > output looks ok though. I've added separate 32-bit and 64-bit helpers= in > order to dispense with the if-else tests. The hw-popcnt versions are = the > opcodes for "popcnt %eax, %eax" and "popcnt %rax, %rax", respectively= , > so %rAX has to be preloaded with the bitmask and the computed value > has to be retrieved from there afterwards. And yes, it looks not that > elegant so I'm open for suggestions. > > The good thing is, this should work on any toolchain since we don't r= ely > on the compiler to know about popcnt and we're protected by CPUID fla= g > so that the hw-popcnt version is used only on processors which suppor= t > it. > > Please take a good look and let me know what do you guys think. > > Thanks. > > -- > =C2=A0arch/x86/include/asm/bitops.h | =C2=A0 =C2=A04 ++ > =C2=A0arch/x86/lib/Makefile =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0= 2 +- > =C2=A0arch/x86/lib/popcnt.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 62 += ++++++++++++++++++++++++++++++++++++++++ > =C2=A03 files changed, 67 insertions(+), 1 deletions(-) > =C2=A0create mode 100644 arch/x86/lib/popcnt.c > > diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bit= ops.h > index 02b47a6..deb5013 100644 > --- a/arch/x86/include/asm/bitops.h > +++ b/arch/x86/include/asm/bitops.h > @@ -434,6 +434,10 @@ static inline int fls(int x) > =C2=A0#endif > =C2=A0 =C2=A0 =C2=A0 =C2=A0return r + 1; > =C2=A0} > + > + > +extern int arch_hweight_long(unsigned long); > + > =C2=A0#endif /* __KERNEL__ */ > > =C2=A0#undef ADDR > diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile > index cffd754..c03fe2d 100644 > --- a/arch/x86/lib/Makefile > +++ b/arch/x86/lib/Makefile > @@ -22,7 +22,7 @@ lib-y +=3D usercopy_$(BITS).o getuser.o putuser.o > =C2=A0lib-y +=3D memcpy_$(BITS).o > =C2=A0lib-$(CONFIG_KPROBES) +=3D insn.o inat.o > > -obj-y +=3D msr.o msr-reg.o msr-reg-export.o > +obj-y +=3D msr.o msr-reg.o msr-reg-export.o popcnt.o > > =C2=A0ifeq ($(CONFIG_X86_32),y) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 obj-y +=3D atomic64_32.o > diff --git a/arch/x86/lib/popcnt.c b/arch/x86/lib/popcnt.c > new file mode 100644 > index 0000000..179a6e8 > --- /dev/null > +++ b/arch/x86/lib/popcnt.c > @@ -0,0 +1,62 @@ > +#include > +#include > + > +int _hweight32(void) > +{ > + =C2=A0 =C2=A0 =C2=A0 unsigned long w; > + > + =C2=A0 =C2=A0 =C2=A0 asm volatile("" : "=3Da" (w)); > + > + =C2=A0 =C2=A0 =C2=A0 return hweight32(w); > + You should just use a normal parameter here (use %rdi for 64-bit). The way you have it written isn't guaranteed to work. GCC could clobber %eax/%rax before you read it, although it is highly unlikely. =46or a good example of how this should be written, look at __mutex_fastpath_lock(). -- Brian Gerst -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html