[PATCH] perf tools: Prevent from BITS_PER_LONG redefinition

[PATCH] perf tools: Prevent from BITS_PER_LONG redefinition

[Frederic Weisbecker]

The system asm/types.h may have defined BITS_PER_LONG already,
depending on the distro.

Check that before defining it from bitops.h wrapper.

Fixes:

In file included from util/include/../../../../include/linux/bitops.h:17,
                 from util/include/linux/bitops.h:8,
                 from ../../lib/hweight.c:2:
util/include/asm/bitops.h:9:1: error: "BITS_PER_LONG" redefined
In file included from util/include/../../../../include/linux/bitops.h:3,
                 from util/include/linux/bitops.h:8,
                 from ../../lib/hweight.c:2:
/usr/include/asm/types.h:32:1: error: this is the location of the previous definition
make: *** [util/hweight.o] Erreur 1

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
---
 tools/perf/util/include/asm/bitops.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/include/asm/bitops.h b/tools/perf/util/include/asm/bitops.h
index 58e9817..d945334 100644
--- a/tools/perf/util/include/asm/bitops.h
+++ b/tools/perf/util/include/asm/bitops.h
@@ -5,8 +5,10 @@
 #include "../../types.h"
 #include <linux/compiler.h>
 
+#ifndef BITS_PER_LONG
 /* CHECKME: Not sure both always match */
 #define BITS_PER_LONG	__WORDSIZE
+#endif
 
 #include "../../../../include/asm-generic/bitops/__fls.h"
 #include "../../../../include/asm-generic/bitops/fls.h"
-- 
1.6.2.3

Re: [PATCH] perf tools: Prevent from BITS_PER_LONG redefinition

[Peter Zijlstra]

On Fri, 2009-12-18 at 23:05 +0100, Frederic Weisbecker wrote:
> +#ifndef BITS_PER_LONG
>  /* CHECKME: Not sure both always match */
>  #define BITS_PER_LONG  __WORDSIZE
> +#endif 

Why use __WORDSIZE if you're not sure?

(8*sizeof(long)) is simple and unambiguous.

Re: [PATCH] perf tools: Prevent from BITS_PER_LONG redefinition

[Frederic Weisbecker]

On Sat, Dec 19, 2009 at 10:26:07AM +0100, Peter Zijlstra wrote:
> On Fri, 2009-12-18 at 23:05 +0100, Frederic Weisbecker wrote:
> > +#ifndef BITS_PER_LONG
> >  /* CHECKME: Not sure both always match */
> >  #define BITS_PER_LONG  __WORDSIZE
> > +#endif 
> 
> Why use __WORDSIZE if you're not sure?
> 
> (8*sizeof(long)) is simple and unambiguous.


Yeah but we need it from the CPP level.
We include such code located in kernel headers:

static __always_inline unsigned long __fls(unsigned long word)
{
	int num = BITS_PER_LONG - 1;

#if BITS_PER_LONG == 64
	if (!(word & (~0ul << 32))) {
		num -= 32;
		word <<= 32;
	}
#endif
	if (!(word & (~0ul << (BITS_PER_LONG-16)))) {
		num -= 16;
		word <<= 16;
	}


And sizeof() is not defined :)

Re: [PATCH] perf tools: Prevent from BITS_PER_LONG redefinition

[Jeremy Fitzhardinge]

On 12/19/2009 06:34 AM, Frederic Weisbecker wrote:
> Yeah but we need it from the CPP level.
> We include such code located in kernel headers:
>
> static __always_inline unsigned long __fls(unsigned long word)
> {
> 	int num = BITS_PER_LONG - 1;
>
> #if BITS_PER_LONG == 64
> 	if (!(word&  (~0ul<<  32))) {
> 		num -= 32;
> 		word<<= 32;
> 	}
> #endif
> 	if (!(word&  (~0ul<<  (BITS_PER_LONG-16)))) {
> 		num -= 16;
> 		word<<= 16;
> 	}
>
>
> And sizeof() is not defined :)
>    

You can use if() with a constant expression instead of #if.

     J

Re: [PATCH] perf tools: Prevent from BITS_PER_LONG redefinition

[Frederic Weisbecker]

On Sat, Dec 19, 2009 at 09:11:20PM -0800, Jeremy Fitzhardinge wrote:
> On 12/19/2009 06:34 AM, Frederic Weisbecker wrote:
>> Yeah but we need it from the CPP level.
>> We include such code located in kernel headers:
>>
>> static __always_inline unsigned long __fls(unsigned long word)
>> {
>> 	int num = BITS_PER_LONG - 1;
>>
>> #if BITS_PER_LONG == 64
>> 	if (!(word&  (~0ul<<  32))) {
>> 		num -= 32;
>> 		word<<= 32;
>> 	}
>> #endif
>> 	if (!(word&  (~0ul<<  (BITS_PER_LONG-16)))) {
>> 		num -= 16;
>> 		word<<= 16;
>> 	}
>>
>>
>> And sizeof() is not defined :)
>>    
>
> You can use if() with a constant expression instead of #if.



I did not write this code. But yes you're right, although I
think CPP is more suitable here because fls() can be called
from fastpath and this conditional build makes one check less
and lesser i-cache footprint.

Re: [PATCH] perf tools: Prevent from BITS_PER_LONG redefinition

[Jeremy Fitzhardinge]

On 12/31/2009 08:45 AM, Frederic Weisbecker wrote:
>> You can use if() with a constant expression instead of #if.
>>      
>
>
> I did not write this code. But yes you're right, although I
> think CPP is more suitable here because fls() can be called
> from fastpath and this conditional build makes one check less
> and lesser i-cache footprint.
>    

Constant if()s are removed at compile time, so there should be no 
runtime overhead.  Constant if()s are generally preferable to #if 
because the compile will statically check the other code branch, even if 
it never gets executed, which helps prevent it from rotting.

     J

Re: [PATCH] perf tools: Prevent from BITS_PER_LONG redefinition

[Frederic Weisbecker]

On Thu, Dec 31, 2009 at 10:09:43PM +1100, Jeremy Fitzhardinge wrote:
> On 12/31/2009 08:45 AM, Frederic Weisbecker wrote:
>>> You can use if() with a constant expression instead of #if.
>>>      
>>
>>
>> I did not write this code. But yes you're right, although I
>> think CPP is more suitable here because fls() can be called
>> from fastpath and this conditional build makes one check less
>> and lesser i-cache footprint.
>>    
>
> Constant if()s are removed at compile time, so there should be no  
> runtime overhead.  Constant if()s are generally preferable to #if  
> because the compile will statically check the other code branch, even if  
> it never gets executed, which helps prevent it from rotting.



Good point! I did not think about compile time optimizations.
I'll try that then. Thanks!