Re: [PATCH v3 7/7] perf expr: Migrate expr ids table to a hashmap

[Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>]

Em Mon, May 18, 2020 at 09:29:06AM -0700, Ian Rogers escreveu:
> I had some issues here too:
> https://lore.kernel.org/lkml/CAEf4BzYxTTND7T7X0dLr2CbkEvUuKtarOeoJYYROefij+qds0w@mail.gmail.com/
> The only reason for the bits/reg.h inclusion is for __WORDSIZE for the
> hash_bits operation. As shown below:

So, to have perf building in all the systems I have test build
containers for I have this in:

tools/include/linux/bitops.h

#include <asm/types.h>
#include <limits.h>
#ifndef __WORDSIZE
#define __WORDSIZE (__SIZEOF_LONG__ * 8)
#endif

With that it works everywhere, Android cross NDK for arm64, older
systems, cross compilers to many arches, Musl libc, uclibc, etc.

So I'm trying, just to check, that this change will make this build
everywhere:

commit ef4c968ccbd52d6a02553719ac7e97f70c65ba47
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Tue May 19 16:26:14 2020 -0300

    WIP
    
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/hashmap.h b/tools/perf/util/hashmap.h
index e823b35e7371..df59fd4fc95b 100644
--- a/tools/perf/util/hashmap.h
+++ b/tools/perf/util/hashmap.h
@@ -10,10 +10,9 @@
 
 #include <stdbool.h>
 #include <stddef.h>
-#ifdef __GLIBC__
-#include <bits/wordsize.h>
-#else
-#include <bits/reg.h>
+#include <limits.h>
+#ifndef __WORDSIZE
+#define __WORDSIZE (__SIZEOF_LONG__ * 8)
 #endif
 
 static inline size_t hash_bits(size_t h, int bits)


> #ifdef __GLIBC__
> #include <bits/wordsize.h>
> #else
> #include <bits/reg.h>
> #endif
> static inline size_t hash_bits(size_t h, int bits)
> {
> /* shuffle bits and return requested number of upper bits */
> return (h * 11400714819323198485llu) >> (__WORDSIZE - bits);
> }
> 
> It'd be possible to change the definition of hash_bits and remove the
> #includes by:
> 
> static inline size_t hash_bits(size_t h, int bits)
> {
> /* shuffle bits and return requested number of upper bits */
> #ifdef __LP64__
>   int shift = 64 - bits;
> #else
>   int shift = 32 - bits;
> #endif
> return (h * 11400714819323198485llu) >> shift;
> }
> 
> Others may have a prefered more portable solution. A separate issue
> with this same function is undefined behavior getting flagged
> (unnecessarily) by sanitizers:
> https://lore.kernel.org/lkml/20200508063954.256593-1-irogers@google.com/
> 
> I was planning to come back to that once we got these changes landed.
> 
> Thanks!
> Ian

-- 

- Arnaldo