diff mbox

[API-NEXT] linux-generic: tm: use odp_hash_crc32c() api to avoid arch issues

Message ID 1449093392-18187-1-git-send-email-bill.fischofer@linaro.org
State Superseded
Headers show

Commit Message

Bill Fischofer Dec. 2, 2015, 9:56 p.m. UTC
Change the internal hash_name_and_kind() function to eliminate the use
of architecture-specific hash instructions. This eliminates build issues
surrounding ARM variants. Future optimizations will use the arch directory
consistent with other ODP modules.

Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
---
 platform/linux-generic/odp_name_table.c | 182 +-------------------------------
 1 file changed, 1 insertion(+), 181 deletions(-)

Comments

Bill Fischofer Dec. 4, 2015, 3:49 a.m. UTC | #1
Ping.  This patch needs a review as it fixes critical bug
https://bugs.linaro.org/show_bug.cgi?id=1930 that is breaking CI for
api-next.

Thanks.

On Wed, Dec 2, 2015 at 3:56 PM, Bill Fischofer <bill.fischofer@linaro.org>
wrote:

> Change the internal hash_name_and_kind() function to eliminate the use

> of architecture-specific hash instructions. This eliminates build issues

> surrounding ARM variants. Future optimizations will use the arch directory

> consistent with other ODP modules.

>

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

>  platform/linux-generic/odp_name_table.c | 182

> +-------------------------------

>  1 file changed, 1 insertion(+), 181 deletions(-)

>

> diff --git a/platform/linux-generic/odp_name_table.c

> b/platform/linux-generic/odp_name_table.c

> index 10a760e..610f034 100644

> --- a/platform/linux-generic/odp_name_table.c

> +++ b/platform/linux-generic/odp_name_table.c

> @@ -48,140 +48,6 @@

>

>  #define SECONDARY_HASH_HISTO_PRINT  1

>

> - /* #define USE_AES */

> -

> -#if defined __x86_64__ || defined __i386__

> -

> -#ifdef USE_AES

> -

> -typedef long long int v2di __attribute__((vector_size(16)));

> -

> -static const v2di HASH_CONST1 = { 0x123456,     0xFEBCDA383   };

> -static const v2di HASH_CONST2 = { 0x493BA3F689, 0x102F5D73A8C };

> -

> -#define PLATFORM_HASH_STATE v2di

> -

> -#define PLATFORM_HASH32_INIT(hash_state, name_len)              \

> -       ({                                                      \

> -               hash_state     = HASH_CONST1;                   \

> -               hash_state[0] ^= name_len;                      \

> -       })

> -

> -#define PLATFORM_HASH32(hash_state, name_word)                     \

> -       ({                                                         \

> -               v2di data;                                         \

> -                                                                  \

> -               data[0]    = name_word;                            \

> -               data[1]    = name_word << 1;                            \

> -               hash_state = __builtin_ia32_aesenc128(hash_state, data); \

> -       })

> -

> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                        \

> -       ({                                                              \

> -               uint64_t result;                                        \

> -               v2di     data;                                          \

> -                                                                       \

> -               data[0]    = name_kind;                                 \

> -               data[1]    = name_kind << 7;                            \

> -               hash_state = __builtin_ia32_aesenc128(hash_state, data); \

> -               hash_state = __builtin_ia32_aesenc128(hash_state,       \

> -                                                     HASH_CONST2);     \

> -               hash_state = __builtin_ia32_aesenc128(hash_state,       \

> -                                                     HASH_CONST1);     \

> -               result     = (uint64_t)hash_state[0] ^ hash_state[1];   \

> -               result     = result ^ result >> 32;                     \

> -               (uint32_t)result;                                       \

> -       })

> -

> -#else

> -

> -#define PLATFORM_HASH_STATE uint64_t

> -

> -#define PLATFORM_HASH32_INIT(hash_state, name_len)    \

> -       ({                                            \

> -               hash_state  = (uint64_t)name_len;     \

> -               hash_state |= hash_state << 8;        \

> -               hash_state |= hash_state << 16;       \

> -               hash_state |= hash_state << 32;               \

> -       })

> -

> -#define PLATFORM_HASH32(hash_state, name_word)                  \

> -       ({                                                      \

> -               uint64_t temp;                                  \

> -                                                                       \

> -               temp        = ((uint64_t)name_word) * 0xFEFDFCF5;       \

> -               hash_state  = hash_state * 0xFF;                        \

> -               hash_state ^= temp ^ (uint64_t)name_word;               \

> -       })

> -

> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                \

> -       ({                                                      \

> -               hash_state ^= (((uint32_t)kind) << 13);         \

> -               hash_state  = hash_state * 0xFEFDFCF5;          \

> -               hash_state  = hash_state ^ hash_state >> 32;    \

> -               hash_state  = hash_state % 0xFEEDDCCBBAA1;      \

> -               hash_state  = hash_state ^ hash_state >> 32;    \

> -               (uint32_t)hash_state;                           \

> -       })

> -

> -#endif

> -

> -#elif defined(__tile_gx__)

> -

> -#define PLATFORM_HASH_STATE  uint32_t

> -

> -#define PLATFORM_HASH32_INIT(hash_state, name_len)              \

> -       ({                                                      \

> -               hash_state = 0xFEFDFCF5;                               \

> -               hash_state = __insn_crc_32_32(hash_state, name_len);   \

> -       })

> -

> -#define PLATFORM_HASH32(hash_state, name_word)                  \

> -       ({                                                             \

> -               hash_state = __insn_crc_32_32(hash_state, name_word);  \

> -       })

> -

> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                \

> -       ({                                                             \

> -               hash_state = __insn_crc_32_32(hash_state, kind);       \

> -               hash_state = __insn_crc_32_32(hash_state, 0xFFFFFFFF); \

> -               hash_state = __insn_crc_32_32(hash_state, 0xFEFDFCF5); \

> -               (uint32_t)hash_state;                                  \

> -       })

> -

> -#elif defined(__arm__) || defined(__aarch64__)

> -

> -#define PLATFORM_HASH_STATE  uint32_t

> -

> -static inline uint32_t __crc32w(uint32_t crc, uint32_t value)

> -{

> -       __asm__("crc32w %w[c], %w[c], %w[v]":[c]"+r"(crc):[v]"r"(value));

> -       return crc;

> -}

> -

> -#define PLATFORM_HASH32_INIT(hash_state, name_len)             \

> -       ({                                                     \

> -               hash_state = 0xFEFDFCF5;                       \

> -               hash_state = __crc32w(hash_state, name_len);   \

> -       })

> -

> -#define PLATFORM_HASH32(hash_state, name_word)                  \

> -       ({                                                      \

> -               __crc32w(hash_state, name_word);                \

> -       })

> -

> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                \

> -       ({                                                      \

> -               hash_state = __crc32w(hash_state, kind);        \

> -               hash_state = __crc32w(hash_state, 0xFFFFFFFF);  \

> -               hash_state = __crc32w(hash_state, 0xFEFDFCF5);  \

> -               (uint32_t)hash_state;                           \

> -       })

> -

> -#else

> -#error "Need to define PLATFORM_DEPENDENT_HASH32 macro"

> -#endif

> -

>  typedef struct name_tbl_entry_s name_tbl_entry_t;

>

>   /* It is important for most platforms that the following struct fit

> within

> @@ -275,53 +141,7 @@ static void aligned_free(void *mem_ptr)

>

>  static uint32_t hash_name_and_kind(const char *name, uint8_t name_kind)

>  {

> -       PLATFORM_HASH_STATE hash_state;

> -       uint32_t            name_len, name_word, hash_value;

> -       uint32_t            bytes[4];

> -

> -       name_len = strlen(name);

> -       PLATFORM_HASH32_INIT(hash_state, name_len);

> -

> -       while (4 <= name_len) {

> -               /* Get the next four characters.  Note that endianness

> doesn't

> -                * matter here!  Also note that this assumes that there is

> -                * either no alignment loading restrictions OR that name is

> -                * 32-bit aligned.  Shouldn't be too hard to add code to

> deal

> -                * with the case when this assumption is false.

> -                */

> -               /* name_word = *((uint32_t *)name); */

> -               bytes[0] = name[0];

> -               bytes[1] = name[1];

> -               bytes[2] = name[2];

> -               bytes[3] = name[3];

> -               name_word = (bytes[3] << 24) | (bytes[2] << 16) |

> -                       (bytes[1] << 8) | bytes[0];

> -               PLATFORM_HASH32(hash_state, name_word);

> -

> -               name_len -= 4;

> -               name     += 4;

> -       }

> -

> -       if (name_len != 0) {

> -               name_word = 0;

> -

> -               if (2 <= name_len) {

> -                       /* name_word = (uint32_t)*((uint16_t *)name); */

> -                       bytes[0] = name[0];

> -                       bytes[1] = name[1];

> -                       name_word |= (bytes[1] << 8) | bytes[0];

> -                       name_len -= 2;

> -                       name     += 2;

> -               }

> -

> -               if (name_len == 1)

> -                       name_word |= ((uint32_t)*name) << 16;

> -

> -               PLATFORM_HASH32(hash_state, name_word);

> -       }

> -

> -       hash_value = PLATFORM_HASH32_FINISH(hash_state, name_kind);

> -       return hash_value;

> +       return odp_hash_crc32c(name, strlen(name), name_kind);

>  }

>

>  static uint32_t linked_list_len(name_tbl_entry_t *name_tbl_entry)

> --

> 2.1.4

>

>
Maxim Uvarov Dec. 4, 2015, 11:52 a.m. UTC | #2
Bill, here is the same problem which I had with mine updated IPC patches.

odp-check says that there is no hash_main binary so test fails.

I think something like that is needed. But it does not help:

+++ b/test/validation/hash/Makefile.am
@@ -3,7 +3,7 @@ include ../Makefile.inc
  noinst_LTLIBRARIES = libtesthash.la
  libtesthash_la_SOURCES = hash.c

-bin_PROGRAMS = hash_main$(EXEEXT)
+test_PROGRAMS = hash_main$(EXEEXT)

CC Anders as autotools expert :)

Maxim.


On 12/04/2015 06:49, Bill Fischofer wrote:
> Ping.  This patch needs a review as it fixes critical bug 
> https://bugs.linaro.org/show_bug.cgi?id=1930 that is breaking CI for 
> api-next.
>
> Thanks.
>
> On Wed, Dec 2, 2015 at 3:56 PM, Bill Fischofer 
> <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> wrote:
>
>     Change the internal hash_name_and_kind() function to eliminate the use
>     of architecture-specific hash instructions. This eliminates build
>     issues
>     surrounding ARM variants. Future optimizations will use the arch
>     directory
>     consistent with other ODP modules.
>
>     Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org
>     <mailto:bill.fischofer@linaro.org>>
>     ---
>      platform/linux-generic/odp_name_table.c | 182
>     +-------------------------------
>      1 file changed, 1 insertion(+), 181 deletions(-)
>
>     diff --git a/platform/linux-generic/odp_name_table.c
>     b/platform/linux-generic/odp_name_table.c
>     index 10a760e..610f034 100644
>     --- a/platform/linux-generic/odp_name_table.c
>     +++ b/platform/linux-generic/odp_name_table.c
>     @@ -48,140 +48,6 @@
>
>      #define SECONDARY_HASH_HISTO_PRINT  1
>
>     - /* #define USE_AES */
>     -
>     -#if defined __x86_64__ || defined __i386__
>     -
>     -#ifdef USE_AES
>     -
>     -typedef long long int v2di __attribute__((vector_size(16)));
>     -
>     -static const v2di HASH_CONST1 = { 0x123456,  0xFEBCDA383   };
>     -static const v2di HASH_CONST2 = { 0x493BA3F689, 0x102F5D73A8C };
>     -
>     -#define PLATFORM_HASH_STATE v2di
>     -
>     -#define PLATFORM_HASH32_INIT(hash_state, name_len)     \
>     -       ({     \
>     -               hash_state     = HASH_CONST1;    \
>     -               hash_state[0] ^= name_len;     \
>     -       })
>     -
>     -#define PLATFORM_HASH32(hash_state, name_word)        \
>     -       ({        \
>     -               v2di data;        \
>     -       \
>     -               data[0]    = name_word;       \
>     -               data[1]    = name_word << 1;                   \
>     -               hash_state = __builtin_ia32_aesenc128(hash_state,
>     data); \
>     -       })
>     -
>     -#define PLATFORM_HASH32_FINISH(hash_state, kind)             \
>     -       ({             \
>     -               uint64_t result;             \
>     -               v2di     data;             \
>     -            \
>     -               data[0]    = name_kind;            \
>     -               data[1]    = name_kind << 7;                   \
>     -               hash_state = __builtin_ia32_aesenc128(hash_state,
>     data); \
>     -               hash_state = __builtin_ia32_aesenc128(hash_state, 
>          \
>     -  HASH_CONST2);     \
>     -               hash_state = __builtin_ia32_aesenc128(hash_state, 
>          \
>     -  HASH_CONST1);     \
>     -               result     = (uint64_t)hash_state[0] ^
>     hash_state[1];   \
>     -               result     = result ^ result >> 32;                  \
>     -               (uint32_t)result;            \
>     -       })
>     -
>     -#else
>     -
>     -#define PLATFORM_HASH_STATE uint64_t
>     -
>     -#define PLATFORM_HASH32_INIT(hash_state, name_len)    \
>     -       ({                                            \
>     -               hash_state  = (uint64_t)name_len;     \
>     -               hash_state |= hash_state << 8; \
>     -               hash_state |= hash_state << 16;  \
>     -               hash_state |= hash_state << 32;        \
>     -       })
>     -
>     -#define PLATFORM_HASH32(hash_state, name_word)     \
>     -       ({     \
>     -               uint64_t temp;     \
>     -            \
>     -               temp        = ((uint64_t)name_word) * 0xFEFDFCF5; 
>          \
>     -               hash_state  = hash_state * 0xFF;             \
>     -               hash_state ^= temp ^ (uint64_t)name_word;            \
>     -       })
>     -
>     -#define PLATFORM_HASH32_FINISH(hash_state, kind)     \
>     -       ({     \
>     -               hash_state ^= (((uint32_t)kind) << 13);         \
>     -               hash_state  = hash_state * 0xFEFDFCF5;     \
>     -               hash_state  = hash_state ^ hash_state >> 32;    \
>     -               hash_state  = hash_state % 0xFEEDDCCBBAA1;     \
>     -               hash_state  = hash_state ^ hash_state >> 32;    \
>     -               (uint32_t)hash_state;    \
>     -       })
>     -
>     -#endif
>     -
>     -#elif defined(__tile_gx__)
>     -
>     -#define PLATFORM_HASH_STATE  uint32_t
>     -
>     -#define PLATFORM_HASH32_INIT(hash_state, name_len)     \
>     -       ({     \
>     -               hash_state = 0xFEFDFCF5;            \
>     -               hash_state = __insn_crc_32_32(hash_state,
>     name_len);   \
>     -       })
>     -
>     -#define PLATFORM_HASH32(hash_state, name_word)     \
>     -       ({            \
>     -               hash_state = __insn_crc_32_32(hash_state,
>     name_word);  \
>     -       })
>     -
>     -#define PLATFORM_HASH32_FINISH(hash_state, kind)     \
>     -       ({            \
>     -               hash_state = __insn_crc_32_32(hash_state, kind); 
>          \
>     -               hash_state = __insn_crc_32_32(hash_state,
>     0xFFFFFFFF); \
>     -               hash_state = __insn_crc_32_32(hash_state,
>     0xFEFDFCF5); \
>     -               (uint32_t)hash_state;           \
>     -       })
>     -
>     -#elif defined(__arm__) || defined(__aarch64__)
>     -
>     -#define PLATFORM_HASH_STATE  uint32_t
>     -
>     -static inline uint32_t __crc32w(uint32_t crc, uint32_t value)
>     -{
>     -       __asm__("crc32w %w[c], %w[c],
>     %w[v]":[c]"+r"(crc):[v]"r"(value));
>     -       return crc;
>     -}
>     -
>     -#define PLATFORM_HASH32_INIT(hash_state, name_len)    \
>     -       ({    \
>     -               hash_state = 0xFEFDFCF5;    \
>     -               hash_state = __crc32w(hash_state, name_len);   \
>     -       })
>     -
>     -#define PLATFORM_HASH32(hash_state, name_word)     \
>     -       ({     \
>     -               __crc32w(hash_state, name_word);     \
>     -       })
>     -
>     -#define PLATFORM_HASH32_FINISH(hash_state, kind)     \
>     -       ({     \
>     -               hash_state = __crc32w(hash_state, kind);     \
>     -               hash_state = __crc32w(hash_state, 0xFFFFFFFF);  \
>     -               hash_state = __crc32w(hash_state, 0xFEFDFCF5);  \
>     -               (uint32_t)hash_state;    \
>     -       })
>     -
>     -#else
>     -#error "Need to define PLATFORM_DEPENDENT_HASH32 macro"
>     -#endif
>     -
>      typedef struct name_tbl_entry_s name_tbl_entry_t;
>
>       /* It is important for most platforms that the following struct
>     fit within
>     @@ -275,53 +141,7 @@ static void aligned_free(void *mem_ptr)
>
>      static uint32_t hash_name_and_kind(const char *name, uint8_t
>     name_kind)
>      {
>     -       PLATFORM_HASH_STATE hash_state;
>     -       uint32_t            name_len, name_word, hash_value;
>     -       uint32_t            bytes[4];
>     -
>     -       name_len = strlen(name);
>     -       PLATFORM_HASH32_INIT(hash_state, name_len);
>     -
>     -       while (4 <= name_len) {
>     -               /* Get the next four characters.  Note that
>     endianness doesn't
>     -                * matter here!  Also note that this assumes that
>     there is
>     -                * either no alignment loading restrictions OR
>     that name is
>     -                * 32-bit aligned.  Shouldn't be too hard to add
>     code to deal
>     -                * with the case when this assumption is false.
>     -                */
>     -               /* name_word = *((uint32_t *)name); */
>     -               bytes[0] = name[0];
>     -               bytes[1] = name[1];
>     -               bytes[2] = name[2];
>     -               bytes[3] = name[3];
>     -               name_word = (bytes[3] << 24) | (bytes[2] << 16) |
>     -                       (bytes[1] << 8) | bytes[0];
>     -               PLATFORM_HASH32(hash_state, name_word);
>     -
>     -               name_len -= 4;
>     -               name     += 4;
>     -       }
>     -
>     -       if (name_len != 0) {
>     -               name_word = 0;
>     -
>     -               if (2 <= name_len) {
>     -                       /* name_word = (uint32_t)*((uint16_t
>     *)name); */
>     -                       bytes[0] = name[0];
>     -                       bytes[1] = name[1];
>     -                       name_word |= (bytes[1] << 8) | bytes[0];
>     -                       name_len -= 2;
>     -                       name     += 2;
>     -               }
>     -
>     -               if (name_len == 1)
>     -                       name_word |= ((uint32_t)*name) << 16;
>     -
>     -               PLATFORM_HASH32(hash_state, name_word);
>     -       }
>     -
>     -       hash_value = PLATFORM_HASH32_FINISH(hash_state, name_kind);
>     -       return hash_value;
>     +       return odp_hash_crc32c(name, strlen(name), name_kind);
>      }
>
>      static uint32_t linked_list_len(name_tbl_entry_t *name_tbl_entry)
>     --
>     2.1.4
>
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
Bill Fischofer Dec. 4, 2015, 1:37 p.m. UTC | #3
Per our discussion a few moments ago, When I run the following:

GIT_BRANCH=api-next PATCH_DIR=$HOME/linaro/tmcrcfix CLEANUP=0 ./build.sh

it executes cleanly for me, so I'm not sure what the problem is.

On Fri, Dec 4, 2015 at 5:52 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> Bill, here is the same problem which I had with mine updated IPC patches.

>

> odp-check says that there is no hash_main binary so test fails.

>

> I think something like that is needed. But it does not help:

>

> +++ b/test/validation/hash/Makefile.am

> @@ -3,7 +3,7 @@ include ../Makefile.inc

>  noinst_LTLIBRARIES = libtesthash.la

>  libtesthash_la_SOURCES = hash.c

>

> -bin_PROGRAMS = hash_main$(EXEEXT)

> +test_PROGRAMS = hash_main$(EXEEXT)

>

> CC Anders as autotools expert :)

>

> Maxim.

>

>

> On 12/04/2015 06:49, Bill Fischofer wrote:

>

>> Ping.  This patch needs a review as it fixes critical bug

>> https://bugs.linaro.org/show_bug.cgi?id=1930 that is breaking CI for

>> api-next.

>>

>> Thanks.

>>

>> On Wed, Dec 2, 2015 at 3:56 PM, Bill Fischofer <bill.fischofer@linaro.org

>> <mailto:bill.fischofer@linaro.org>> wrote:

>>

>>     Change the internal hash_name_and_kind() function to eliminate the use

>>     of architecture-specific hash instructions. This eliminates build

>>     issues

>>     surrounding ARM variants. Future optimizations will use the arch

>>     directory

>>     consistent with other ODP modules.

>>

>>     Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org

>>     <mailto:bill.fischofer@linaro.org>>

>>

>>     ---

>>      platform/linux-generic/odp_name_table.c | 182

>>     +-------------------------------

>>      1 file changed, 1 insertion(+), 181 deletions(-)

>>

>>     diff --git a/platform/linux-generic/odp_name_table.c

>>     b/platform/linux-generic/odp_name_table.c

>>     index 10a760e..610f034 100644

>>     --- a/platform/linux-generic/odp_name_table.c

>>     +++ b/platform/linux-generic/odp_name_table.c

>>     @@ -48,140 +48,6 @@

>>

>>      #define SECONDARY_HASH_HISTO_PRINT  1

>>

>>     - /* #define USE_AES */

>>     -

>>     -#if defined __x86_64__ || defined __i386__

>>     -

>>     -#ifdef USE_AES

>>     -

>>     -typedef long long int v2di __attribute__((vector_size(16)));

>>     -

>>     -static const v2di HASH_CONST1 = { 0x123456,  0xFEBCDA383   };

>>     -static const v2di HASH_CONST2 = { 0x493BA3F689, 0x102F5D73A8C };

>>     -

>>     -#define PLATFORM_HASH_STATE v2di

>>     -

>>     -#define PLATFORM_HASH32_INIT(hash_state, name_len)     \

>>     -       ({     \

>>     -               hash_state     = HASH_CONST1;    \

>>     -               hash_state[0] ^= name_len;     \

>>     -       })

>>     -

>>     -#define PLATFORM_HASH32(hash_state, name_word)        \

>>     -       ({        \

>>     -               v2di data;        \

>>     -       \

>>     -               data[0]    = name_word;       \

>>     -               data[1]    = name_word << 1;                   \

>>     -               hash_state = __builtin_ia32_aesenc128(hash_state,

>>     data); \

>>     -       })

>>     -

>>     -#define PLATFORM_HASH32_FINISH(hash_state, kind)             \

>>     -       ({             \

>>     -               uint64_t result;             \

>>     -               v2di     data;             \

>>     -            \

>>     -               data[0]    = name_kind;            \

>>     -               data[1]    = name_kind << 7;                   \

>>     -               hash_state = __builtin_ia32_aesenc128(hash_state,

>>     data); \

>>     -               hash_state = __builtin_ia32_aesenc128(hash_state,

>>       \

>>     -  HASH_CONST2);     \

>>     -               hash_state = __builtin_ia32_aesenc128(hash_state,

>>       \

>>     -  HASH_CONST1);     \

>>     -               result     = (uint64_t)hash_state[0] ^

>>     hash_state[1];   \

>>     -               result     = result ^ result >> 32;                  \

>>     -               (uint32_t)result;            \

>>     -       })

>>     -

>>     -#else

>>     -

>>     -#define PLATFORM_HASH_STATE uint64_t

>>     -

>>     -#define PLATFORM_HASH32_INIT(hash_state, name_len)    \

>>     -       ({                                            \

>>     -               hash_state  = (uint64_t)name_len;     \

>>     -               hash_state |= hash_state << 8; \

>>     -               hash_state |= hash_state << 16;  \

>>     -               hash_state |= hash_state << 32;        \

>>     -       })

>>     -

>>     -#define PLATFORM_HASH32(hash_state, name_word)     \

>>     -       ({     \

>>     -               uint64_t temp;     \

>>     -            \

>>     -               temp        = ((uint64_t)name_word) * 0xFEFDFCF5;

>>       \

>>     -               hash_state  = hash_state * 0xFF;             \

>>     -               hash_state ^= temp ^ (uint64_t)name_word;            \

>>     -       })

>>     -

>>     -#define PLATFORM_HASH32_FINISH(hash_state, kind)     \

>>     -       ({     \

>>     -               hash_state ^= (((uint32_t)kind) << 13);         \

>>     -               hash_state  = hash_state * 0xFEFDFCF5;     \

>>     -               hash_state  = hash_state ^ hash_state >> 32;    \

>>     -               hash_state  = hash_state % 0xFEEDDCCBBAA1;     \

>>     -               hash_state  = hash_state ^ hash_state >> 32;    \

>>     -               (uint32_t)hash_state;    \

>>     -       })

>>     -

>>     -#endif

>>     -

>>     -#elif defined(__tile_gx__)

>>     -

>>     -#define PLATFORM_HASH_STATE  uint32_t

>>     -

>>     -#define PLATFORM_HASH32_INIT(hash_state, name_len)     \

>>     -       ({     \

>>     -               hash_state = 0xFEFDFCF5;            \

>>     -               hash_state = __insn_crc_32_32(hash_state,

>>     name_len);   \

>>     -       })

>>     -

>>     -#define PLATFORM_HASH32(hash_state, name_word)     \

>>     -       ({            \

>>     -               hash_state = __insn_crc_32_32(hash_state,

>>     name_word);  \

>>     -       })

>>     -

>>     -#define PLATFORM_HASH32_FINISH(hash_state, kind)     \

>>     -       ({            \

>>     -               hash_state = __insn_crc_32_32(hash_state, kind);

>>     \

>>     -               hash_state = __insn_crc_32_32(hash_state,

>>     0xFFFFFFFF); \

>>     -               hash_state = __insn_crc_32_32(hash_state,

>>     0xFEFDFCF5); \

>>     -               (uint32_t)hash_state;           \

>>     -       })

>>     -

>>     -#elif defined(__arm__) || defined(__aarch64__)

>>     -

>>     -#define PLATFORM_HASH_STATE  uint32_t

>>     -

>>     -static inline uint32_t __crc32w(uint32_t crc, uint32_t value)

>>     -{

>>     -       __asm__("crc32w %w[c], %w[c],

>>     %w[v]":[c]"+r"(crc):[v]"r"(value));

>>     -       return crc;

>>     -}

>>     -

>>     -#define PLATFORM_HASH32_INIT(hash_state, name_len)    \

>>     -       ({    \

>>     -               hash_state = 0xFEFDFCF5;    \

>>     -               hash_state = __crc32w(hash_state, name_len);   \

>>     -       })

>>     -

>>     -#define PLATFORM_HASH32(hash_state, name_word)     \

>>     -       ({     \

>>     -               __crc32w(hash_state, name_word);     \

>>     -       })

>>     -

>>     -#define PLATFORM_HASH32_FINISH(hash_state, kind)     \

>>     -       ({     \

>>     -               hash_state = __crc32w(hash_state, kind);     \

>>     -               hash_state = __crc32w(hash_state, 0xFFFFFFFF);  \

>>     -               hash_state = __crc32w(hash_state, 0xFEFDFCF5);  \

>>     -               (uint32_t)hash_state;    \

>>     -       })

>>     -

>>     -#else

>>     -#error "Need to define PLATFORM_DEPENDENT_HASH32 macro"

>>     -#endif

>>     -

>>      typedef struct name_tbl_entry_s name_tbl_entry_t;

>>

>>       /* It is important for most platforms that the following struct

>>     fit within

>>     @@ -275,53 +141,7 @@ static void aligned_free(void *mem_ptr)

>>

>>      static uint32_t hash_name_and_kind(const char *name, uint8_t

>>     name_kind)

>>      {

>>     -       PLATFORM_HASH_STATE hash_state;

>>     -       uint32_t            name_len, name_word, hash_value;

>>     -       uint32_t            bytes[4];

>>     -

>>     -       name_len = strlen(name);

>>     -       PLATFORM_HASH32_INIT(hash_state, name_len);

>>     -

>>     -       while (4 <= name_len) {

>>     -               /* Get the next four characters.  Note that

>>     endianness doesn't

>>     -                * matter here!  Also note that this assumes that

>>     there is

>>     -                * either no alignment loading restrictions OR

>>     that name is

>>     -                * 32-bit aligned.  Shouldn't be too hard to add

>>     code to deal

>>     -                * with the case when this assumption is false.

>>     -                */

>>     -               /* name_word = *((uint32_t *)name); */

>>     -               bytes[0] = name[0];

>>     -               bytes[1] = name[1];

>>     -               bytes[2] = name[2];

>>     -               bytes[3] = name[3];

>>     -               name_word = (bytes[3] << 24) | (bytes[2] << 16) |

>>     -                       (bytes[1] << 8) | bytes[0];

>>     -               PLATFORM_HASH32(hash_state, name_word);

>>     -

>>     -               name_len -= 4;

>>     -               name     += 4;

>>     -       }

>>     -

>>     -       if (name_len != 0) {

>>     -               name_word = 0;

>>     -

>>     -               if (2 <= name_len) {

>>     -                       /* name_word = (uint32_t)*((uint16_t

>>     *)name); */

>>     -                       bytes[0] = name[0];

>>     -                       bytes[1] = name[1];

>>     -                       name_word |= (bytes[1] << 8) | bytes[0];

>>     -                       name_len -= 2;

>>     -                       name     += 2;

>>     -               }

>>     -

>>     -               if (name_len == 1)

>>     -                       name_word |= ((uint32_t)*name) << 16;

>>     -

>>     -               PLATFORM_HASH32(hash_state, name_word);

>>     -       }

>>     -

>>     -       hash_value = PLATFORM_HASH32_FINISH(hash_state, name_kind);

>>     -       return hash_value;

>>     +       return odp_hash_crc32c(name, strlen(name), name_kind);

>>      }

>>

>>      static uint32_t linked_list_len(name_tbl_entry_t *name_tbl_entry)

>>     --

>>     2.1.4

>>

>>

>>

>>

>> _______________________________________________

>> lng-odp mailing list

>> lng-odp@lists.linaro.org

>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>

>

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>
Maxim Uvarov Dec. 8, 2015, 3:35 p.m. UTC | #4
Merged this patch.

Now only issue left is fix:
hash_main: no such file
I think something wrong in Makefile.am, not sure if CI also captures 
that issues,
but we will see. That is different issue.

Maxim.


On 12/03/2015 00:56, Bill Fischofer wrote:
> Change the internal hash_name_and_kind() function to eliminate the use
> of architecture-specific hash instructions. This eliminates build issues
> surrounding ARM variants. Future optimizations will use the arch directory
> consistent with other ODP modules.
>
> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> ---
>   platform/linux-generic/odp_name_table.c | 182 +-------------------------------
>   1 file changed, 1 insertion(+), 181 deletions(-)
>
> diff --git a/platform/linux-generic/odp_name_table.c b/platform/linux-generic/odp_name_table.c
> index 10a760e..610f034 100644
> --- a/platform/linux-generic/odp_name_table.c
> +++ b/platform/linux-generic/odp_name_table.c
> @@ -48,140 +48,6 @@
>   
>   #define SECONDARY_HASH_HISTO_PRINT  1
>   
> - /* #define USE_AES */
> -
> -#if defined __x86_64__ || defined __i386__
> -
> -#ifdef USE_AES
> -
> -typedef long long int v2di __attribute__((vector_size(16)));
> -
> -static const v2di HASH_CONST1 = { 0x123456,     0xFEBCDA383   };
> -static const v2di HASH_CONST2 = { 0x493BA3F689, 0x102F5D73A8C };
> -
> -#define PLATFORM_HASH_STATE v2di
> -
> -#define PLATFORM_HASH32_INIT(hash_state, name_len)              \
> -	({                                                      \
> -		hash_state     = HASH_CONST1;                   \
> -		hash_state[0] ^= name_len;                      \
> -	})
> -
> -#define PLATFORM_HASH32(hash_state, name_word)                     \
> -	({                                                         \
> -		v2di data;                                         \
> -								   \
> -		data[0]    = name_word;                            \
> -		data[1]    = name_word << 1;				\
> -		hash_state = __builtin_ia32_aesenc128(hash_state, data); \
> -	})
> -
> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                        \
> -	({                                                              \
> -		uint64_t result;                                        \
> -		v2di     data;                                          \
> -									\
> -		data[0]    = name_kind;                                 \
> -		data[1]    = name_kind << 7;                            \
> -		hash_state = __builtin_ia32_aesenc128(hash_state, data); \
> -		hash_state = __builtin_ia32_aesenc128(hash_state,       \
> -						      HASH_CONST2);     \
> -		hash_state = __builtin_ia32_aesenc128(hash_state,       \
> -						      HASH_CONST1);     \
> -		result     = (uint64_t)hash_state[0] ^ hash_state[1];   \
> -		result     = result ^ result >> 32;                     \
> -		(uint32_t)result;                                       \
> -	})
> -
> -#else
> -
> -#define PLATFORM_HASH_STATE uint64_t
> -
> -#define PLATFORM_HASH32_INIT(hash_state, name_len)    \
> -	({					      \
> -		hash_state  = (uint64_t)name_len;     \
> -		hash_state |= hash_state << 8;	      \
> -		hash_state |= hash_state << 16;	      \
> -		hash_state |= hash_state << 32;		      \
> -	})
> -
> -#define PLATFORM_HASH32(hash_state, name_word)                  \
> -	({                                                      \
> -		uint64_t temp;                                  \
> -									\
> -		temp        = ((uint64_t)name_word) * 0xFEFDFCF5;       \
> -		hash_state  = hash_state * 0xFF;                        \
> -		hash_state ^= temp ^ (uint64_t)name_word;               \
> -	})
> -
> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                \
> -	({                                                      \
> -		hash_state ^= (((uint32_t)kind) << 13);         \
> -		hash_state  = hash_state * 0xFEFDFCF5;          \
> -		hash_state  = hash_state ^ hash_state >> 32;    \
> -		hash_state  = hash_state % 0xFEEDDCCBBAA1;      \
> -		hash_state  = hash_state ^ hash_state >> 32;    \
> -		(uint32_t)hash_state;                           \
> -	})
> -
> -#endif
> -
> -#elif defined(__tile_gx__)
> -
> -#define PLATFORM_HASH_STATE  uint32_t
> -
> -#define PLATFORM_HASH32_INIT(hash_state, name_len)              \
> -	({                                                      \
> -		hash_state = 0xFEFDFCF5;			       \
> -		hash_state = __insn_crc_32_32(hash_state, name_len);   \
> -	})
> -
> -#define PLATFORM_HASH32(hash_state, name_word)                  \
> -	({							       \
> -		hash_state = __insn_crc_32_32(hash_state, name_word);  \
> -	})
> -
> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                \
> -	({							       \
> -		hash_state = __insn_crc_32_32(hash_state, kind);       \
> -		hash_state = __insn_crc_32_32(hash_state, 0xFFFFFFFF); \
> -		hash_state = __insn_crc_32_32(hash_state, 0xFEFDFCF5); \
> -		(uint32_t)hash_state;				       \
> -	})
> -
> -#elif defined(__arm__) || defined(__aarch64__)
> -
> -#define PLATFORM_HASH_STATE  uint32_t
> -
> -static inline uint32_t __crc32w(uint32_t crc, uint32_t value)
> -{
> -	__asm__("crc32w %w[c], %w[c], %w[v]":[c]"+r"(crc):[v]"r"(value));
> -	return crc;
> -}
> -
> -#define PLATFORM_HASH32_INIT(hash_state, name_len)             \
> -	({						       \
> -		hash_state = 0xFEFDFCF5;		       \
> -		hash_state = __crc32w(hash_state, name_len);   \
> -	})
> -
> -#define PLATFORM_HASH32(hash_state, name_word)                  \
> -	({                                                      \
> -		__crc32w(hash_state, name_word);		\
> -	})
> -
> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                \
> -	({                                                      \
> -		hash_state = __crc32w(hash_state, kind);        \
> -		hash_state = __crc32w(hash_state, 0xFFFFFFFF);  \
> -		hash_state = __crc32w(hash_state, 0xFEFDFCF5);  \
> -		(uint32_t)hash_state;                           \
> -	})
> -
> -#else
> -#error "Need to define PLATFORM_DEPENDENT_HASH32 macro"
> -#endif
> -
>   typedef struct name_tbl_entry_s name_tbl_entry_t;
>   
>    /* It is important for most platforms that the following struct fit within
> @@ -275,53 +141,7 @@ static void aligned_free(void *mem_ptr)
>   
>   static uint32_t hash_name_and_kind(const char *name, uint8_t name_kind)
>   {
> -	PLATFORM_HASH_STATE hash_state;
> -	uint32_t            name_len, name_word, hash_value;
> -	uint32_t            bytes[4];
> -
> -	name_len = strlen(name);
> -	PLATFORM_HASH32_INIT(hash_state, name_len);
> -
> -	while (4 <= name_len) {
> -		/* Get the next four characters.  Note that endianness doesn't
> -		 * matter here!  Also note that this assumes that there is
> -		 * either no alignment loading restrictions OR that name is
> -		 * 32-bit aligned.  Shouldn't be too hard to add code to deal
> -		 * with the case when this assumption is false.
> -		 */
> -		/* name_word = *((uint32_t *)name); */
> -		bytes[0] = name[0];
> -		bytes[1] = name[1];
> -		bytes[2] = name[2];
> -		bytes[3] = name[3];
> -		name_word = (bytes[3] << 24) | (bytes[2] << 16) |
> -			(bytes[1] << 8) | bytes[0];
> -		PLATFORM_HASH32(hash_state, name_word);
> -
> -		name_len -= 4;
> -		name     += 4;
> -	}
> -
> -	if (name_len != 0) {
> -		name_word = 0;
> -
> -		if (2 <= name_len) {
> -			/* name_word = (uint32_t)*((uint16_t *)name); */
> -			bytes[0] = name[0];
> -			bytes[1] = name[1];
> -			name_word |= (bytes[1] << 8) | bytes[0];
> -			name_len -= 2;
> -			name     += 2;
> -		}
> -
> -		if (name_len == 1)
> -			name_word |= ((uint32_t)*name) << 16;
> -
> -		PLATFORM_HASH32(hash_state, name_word);
> -	}
> -
> -	hash_value = PLATFORM_HASH32_FINISH(hash_state, name_kind);
> -	return hash_value;
> +	return odp_hash_crc32c(name, strlen(name), name_kind);
>   }
>   
>   static uint32_t linked_list_len(name_tbl_entry_t *name_tbl_entry)
Bill Fischofer Dec. 8, 2015, 5:39 p.m. UTC | #5
hash_main is the binary output from compiling the hash CUnit test.  That's
should have nothing to do with TM.

On Tue, Dec 8, 2015 at 9:35 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> Merged this patch.

>

> Now only issue left is fix:

> hash_main: no such file

> I think something wrong in Makefile.am, not sure if CI also captures that

> issues,

> but we will see. That is different issue.

>

> Maxim.

>

>

>

> On 12/03/2015 00:56, Bill Fischofer wrote:

>

>> Change the internal hash_name_and_kind() function to eliminate the use

>> of architecture-specific hash instructions. This eliminates build issues

>> surrounding ARM variants. Future optimizations will use the arch directory

>> consistent with other ODP modules.

>>

>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>> ---

>>   platform/linux-generic/odp_name_table.c | 182

>> +-------------------------------

>>   1 file changed, 1 insertion(+), 181 deletions(-)

>>

>> diff --git a/platform/linux-generic/odp_name_table.c

>> b/platform/linux-generic/odp_name_table.c

>> index 10a760e..610f034 100644

>> --- a/platform/linux-generic/odp_name_table.c

>> +++ b/platform/linux-generic/odp_name_table.c

>> @@ -48,140 +48,6 @@

>>     #define SECONDARY_HASH_HISTO_PRINT  1

>>   - /* #define USE_AES */

>> -

>> -#if defined __x86_64__ || defined __i386__

>> -

>> -#ifdef USE_AES

>> -

>> -typedef long long int v2di __attribute__((vector_size(16)));

>> -

>> -static const v2di HASH_CONST1 = { 0x123456,     0xFEBCDA383   };

>> -static const v2di HASH_CONST2 = { 0x493BA3F689, 0x102F5D73A8C };

>> -

>> -#define PLATFORM_HASH_STATE v2di

>> -

>> -#define PLATFORM_HASH32_INIT(hash_state, name_len)              \

>> -       ({                                                      \

>> -               hash_state     = HASH_CONST1;                   \

>> -               hash_state[0] ^= name_len;                      \

>> -       })

>> -

>> -#define PLATFORM_HASH32(hash_state, name_word)                     \

>> -       ({                                                         \

>> -               v2di data;                                         \

>> -                                                                  \

>> -               data[0]    = name_word;                            \

>> -               data[1]    = name_word << 1;                            \

>> -               hash_state = __builtin_ia32_aesenc128(hash_state, data); \

>> -       })

>> -

>> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                        \

>> -       ({                                                              \

>> -               uint64_t result;                                        \

>> -               v2di     data;                                          \

>> -                                                                       \

>> -               data[0]    = name_kind;                                 \

>> -               data[1]    = name_kind << 7;                            \

>> -               hash_state = __builtin_ia32_aesenc128(hash_state, data); \

>> -               hash_state = __builtin_ia32_aesenc128(hash_state,       \

>> -                                                     HASH_CONST2);     \

>> -               hash_state = __builtin_ia32_aesenc128(hash_state,       \

>> -                                                     HASH_CONST1);     \

>> -               result     = (uint64_t)hash_state[0] ^ hash_state[1];   \

>> -               result     = result ^ result >> 32;                     \

>> -               (uint32_t)result;                                       \

>> -       })

>> -

>> -#else

>> -

>> -#define PLATFORM_HASH_STATE uint64_t

>> -

>> -#define PLATFORM_HASH32_INIT(hash_state, name_len)    \

>> -       ({                                            \

>> -               hash_state  = (uint64_t)name_len;     \

>> -               hash_state |= hash_state << 8;        \

>> -               hash_state |= hash_state << 16;       \

>> -               hash_state |= hash_state << 32;               \

>> -       })

>> -

>> -#define PLATFORM_HASH32(hash_state, name_word)                  \

>> -       ({                                                      \

>> -               uint64_t temp;                                  \

>> -                                                                       \

>> -               temp        = ((uint64_t)name_word) * 0xFEFDFCF5;       \

>> -               hash_state  = hash_state * 0xFF;                        \

>> -               hash_state ^= temp ^ (uint64_t)name_word;               \

>> -       })

>> -

>> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                \

>> -       ({                                                      \

>> -               hash_state ^= (((uint32_t)kind) << 13);         \

>> -               hash_state  = hash_state * 0xFEFDFCF5;          \

>> -               hash_state  = hash_state ^ hash_state >> 32;    \

>> -               hash_state  = hash_state % 0xFEEDDCCBBAA1;      \

>> -               hash_state  = hash_state ^ hash_state >> 32;    \

>> -               (uint32_t)hash_state;                           \

>> -       })

>> -

>> -#endif

>> -

>> -#elif defined(__tile_gx__)

>> -

>> -#define PLATFORM_HASH_STATE  uint32_t

>> -

>> -#define PLATFORM_HASH32_INIT(hash_state, name_len)              \

>> -       ({                                                      \

>> -               hash_state = 0xFEFDFCF5;                               \

>> -               hash_state = __insn_crc_32_32(hash_state, name_len);   \

>> -       })

>> -

>> -#define PLATFORM_HASH32(hash_state, name_word)                  \

>> -       ({                                                             \

>> -               hash_state = __insn_crc_32_32(hash_state, name_word);  \

>> -       })

>> -

>> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                \

>> -       ({                                                             \

>> -               hash_state = __insn_crc_32_32(hash_state, kind);       \

>> -               hash_state = __insn_crc_32_32(hash_state, 0xFFFFFFFF); \

>> -               hash_state = __insn_crc_32_32(hash_state, 0xFEFDFCF5); \

>> -               (uint32_t)hash_state;                                  \

>> -       })

>> -

>> -#elif defined(__arm__) || defined(__aarch64__)

>> -

>> -#define PLATFORM_HASH_STATE  uint32_t

>> -

>> -static inline uint32_t __crc32w(uint32_t crc, uint32_t value)

>> -{

>> -       __asm__("crc32w %w[c], %w[c], %w[v]":[c]"+r"(crc):[v]"r"(value));

>> -       return crc;

>> -}

>> -

>> -#define PLATFORM_HASH32_INIT(hash_state, name_len)             \

>> -       ({                                                     \

>> -               hash_state = 0xFEFDFCF5;                       \

>> -               hash_state = __crc32w(hash_state, name_len);   \

>> -       })

>> -

>> -#define PLATFORM_HASH32(hash_state, name_word)                  \

>> -       ({                                                      \

>> -               __crc32w(hash_state, name_word);                \

>> -       })

>> -

>> -#define PLATFORM_HASH32_FINISH(hash_state, kind)                \

>> -       ({                                                      \

>> -               hash_state = __crc32w(hash_state, kind);        \

>> -               hash_state = __crc32w(hash_state, 0xFFFFFFFF);  \

>> -               hash_state = __crc32w(hash_state, 0xFEFDFCF5);  \

>> -               (uint32_t)hash_state;                           \

>> -       })

>> -

>> -#else

>> -#error "Need to define PLATFORM_DEPENDENT_HASH32 macro"

>> -#endif

>> -

>>   typedef struct name_tbl_entry_s name_tbl_entry_t;

>>      /* It is important for most platforms that the following struct fit

>> within

>> @@ -275,53 +141,7 @@ static void aligned_free(void *mem_ptr)

>>     static uint32_t hash_name_and_kind(const char *name, uint8_t

>> name_kind)

>>   {

>> -       PLATFORM_HASH_STATE hash_state;

>> -       uint32_t            name_len, name_word, hash_value;

>> -       uint32_t            bytes[4];

>> -

>> -       name_len = strlen(name);

>> -       PLATFORM_HASH32_INIT(hash_state, name_len);

>> -

>> -       while (4 <= name_len) {

>> -               /* Get the next four characters.  Note that endianness

>> doesn't

>> -                * matter here!  Also note that this assumes that there is

>> -                * either no alignment loading restrictions OR that name

>> is

>> -                * 32-bit aligned.  Shouldn't be too hard to add code to

>> deal

>> -                * with the case when this assumption is false.

>> -                */

>> -               /* name_word = *((uint32_t *)name); */

>> -               bytes[0] = name[0];

>> -               bytes[1] = name[1];

>> -               bytes[2] = name[2];

>> -               bytes[3] = name[3];

>> -               name_word = (bytes[3] << 24) | (bytes[2] << 16) |

>> -                       (bytes[1] << 8) | bytes[0];

>> -               PLATFORM_HASH32(hash_state, name_word);

>> -

>> -               name_len -= 4;

>> -               name     += 4;

>> -       }

>> -

>> -       if (name_len != 0) {

>> -               name_word = 0;

>> -

>> -               if (2 <= name_len) {

>> -                       /* name_word = (uint32_t)*((uint16_t *)name); */

>> -                       bytes[0] = name[0];

>> -                       bytes[1] = name[1];

>> -                       name_word |= (bytes[1] << 8) | bytes[0];

>> -                       name_len -= 2;

>> -                       name     += 2;

>> -               }

>> -

>> -               if (name_len == 1)

>> -                       name_word |= ((uint32_t)*name) << 16;

>> -

>> -               PLATFORM_HASH32(hash_state, name_word);

>> -       }

>> -

>> -       hash_value = PLATFORM_HASH32_FINISH(hash_state, name_kind);

>> -       return hash_value;

>> +       return odp_hash_crc32c(name, strlen(name), name_kind);

>>   }

>>     static uint32_t linked_list_len(name_tbl_entry_t *name_tbl_entry)

>>

>

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>
diff mbox

Patch

diff --git a/platform/linux-generic/odp_name_table.c b/platform/linux-generic/odp_name_table.c
index 10a760e..610f034 100644
--- a/platform/linux-generic/odp_name_table.c
+++ b/platform/linux-generic/odp_name_table.c
@@ -48,140 +48,6 @@ 
 
 #define SECONDARY_HASH_HISTO_PRINT  1
 
- /* #define USE_AES */
-
-#if defined __x86_64__ || defined __i386__
-
-#ifdef USE_AES
-
-typedef long long int v2di __attribute__((vector_size(16)));
-
-static const v2di HASH_CONST1 = { 0x123456,     0xFEBCDA383   };
-static const v2di HASH_CONST2 = { 0x493BA3F689, 0x102F5D73A8C };
-
-#define PLATFORM_HASH_STATE v2di
-
-#define PLATFORM_HASH32_INIT(hash_state, name_len)              \
-	({                                                      \
-		hash_state     = HASH_CONST1;                   \
-		hash_state[0] ^= name_len;                      \
-	})
-
-#define PLATFORM_HASH32(hash_state, name_word)                     \
-	({                                                         \
-		v2di data;                                         \
-								   \
-		data[0]    = name_word;                            \
-		data[1]    = name_word << 1;				\
-		hash_state = __builtin_ia32_aesenc128(hash_state, data); \
-	})
-
-#define PLATFORM_HASH32_FINISH(hash_state, kind)                        \
-	({                                                              \
-		uint64_t result;                                        \
-		v2di     data;                                          \
-									\
-		data[0]    = name_kind;                                 \
-		data[1]    = name_kind << 7;                            \
-		hash_state = __builtin_ia32_aesenc128(hash_state, data); \
-		hash_state = __builtin_ia32_aesenc128(hash_state,       \
-						      HASH_CONST2);     \
-		hash_state = __builtin_ia32_aesenc128(hash_state,       \
-						      HASH_CONST1);     \
-		result     = (uint64_t)hash_state[0] ^ hash_state[1];   \
-		result     = result ^ result >> 32;                     \
-		(uint32_t)result;                                       \
-	})
-
-#else
-
-#define PLATFORM_HASH_STATE uint64_t
-
-#define PLATFORM_HASH32_INIT(hash_state, name_len)    \
-	({					      \
-		hash_state  = (uint64_t)name_len;     \
-		hash_state |= hash_state << 8;	      \
-		hash_state |= hash_state << 16;	      \
-		hash_state |= hash_state << 32;		      \
-	})
-
-#define PLATFORM_HASH32(hash_state, name_word)                  \
-	({                                                      \
-		uint64_t temp;                                  \
-									\
-		temp        = ((uint64_t)name_word) * 0xFEFDFCF5;       \
-		hash_state  = hash_state * 0xFF;                        \
-		hash_state ^= temp ^ (uint64_t)name_word;               \
-	})
-
-#define PLATFORM_HASH32_FINISH(hash_state, kind)                \
-	({                                                      \
-		hash_state ^= (((uint32_t)kind) << 13);         \
-		hash_state  = hash_state * 0xFEFDFCF5;          \
-		hash_state  = hash_state ^ hash_state >> 32;    \
-		hash_state  = hash_state % 0xFEEDDCCBBAA1;      \
-		hash_state  = hash_state ^ hash_state >> 32;    \
-		(uint32_t)hash_state;                           \
-	})
-
-#endif
-
-#elif defined(__tile_gx__)
-
-#define PLATFORM_HASH_STATE  uint32_t
-
-#define PLATFORM_HASH32_INIT(hash_state, name_len)              \
-	({                                                      \
-		hash_state = 0xFEFDFCF5;			       \
-		hash_state = __insn_crc_32_32(hash_state, name_len);   \
-	})
-
-#define PLATFORM_HASH32(hash_state, name_word)                  \
-	({							       \
-		hash_state = __insn_crc_32_32(hash_state, name_word);  \
-	})
-
-#define PLATFORM_HASH32_FINISH(hash_state, kind)                \
-	({							       \
-		hash_state = __insn_crc_32_32(hash_state, kind);       \
-		hash_state = __insn_crc_32_32(hash_state, 0xFFFFFFFF); \
-		hash_state = __insn_crc_32_32(hash_state, 0xFEFDFCF5); \
-		(uint32_t)hash_state;				       \
-	})
-
-#elif defined(__arm__) || defined(__aarch64__)
-
-#define PLATFORM_HASH_STATE  uint32_t
-
-static inline uint32_t __crc32w(uint32_t crc, uint32_t value)
-{
-	__asm__("crc32w %w[c], %w[c], %w[v]":[c]"+r"(crc):[v]"r"(value));
-	return crc;
-}
-
-#define PLATFORM_HASH32_INIT(hash_state, name_len)             \
-	({						       \
-		hash_state = 0xFEFDFCF5;		       \
-		hash_state = __crc32w(hash_state, name_len);   \
-	})
-
-#define PLATFORM_HASH32(hash_state, name_word)                  \
-	({                                                      \
-		__crc32w(hash_state, name_word);		\
-	})
-
-#define PLATFORM_HASH32_FINISH(hash_state, kind)                \
-	({                                                      \
-		hash_state = __crc32w(hash_state, kind);        \
-		hash_state = __crc32w(hash_state, 0xFFFFFFFF);  \
-		hash_state = __crc32w(hash_state, 0xFEFDFCF5);  \
-		(uint32_t)hash_state;                           \
-	})
-
-#else
-#error "Need to define PLATFORM_DEPENDENT_HASH32 macro"
-#endif
-
 typedef struct name_tbl_entry_s name_tbl_entry_t;
 
  /* It is important for most platforms that the following struct fit within
@@ -275,53 +141,7 @@  static void aligned_free(void *mem_ptr)
 
 static uint32_t hash_name_and_kind(const char *name, uint8_t name_kind)
 {
-	PLATFORM_HASH_STATE hash_state;
-	uint32_t            name_len, name_word, hash_value;
-	uint32_t            bytes[4];
-
-	name_len = strlen(name);
-	PLATFORM_HASH32_INIT(hash_state, name_len);
-
-	while (4 <= name_len) {
-		/* Get the next four characters.  Note that endianness doesn't
-		 * matter here!  Also note that this assumes that there is
-		 * either no alignment loading restrictions OR that name is
-		 * 32-bit aligned.  Shouldn't be too hard to add code to deal
-		 * with the case when this assumption is false.
-		 */
-		/* name_word = *((uint32_t *)name); */
-		bytes[0] = name[0];
-		bytes[1] = name[1];
-		bytes[2] = name[2];
-		bytes[3] = name[3];
-		name_word = (bytes[3] << 24) | (bytes[2] << 16) |
-			(bytes[1] << 8) | bytes[0];
-		PLATFORM_HASH32(hash_state, name_word);
-
-		name_len -= 4;
-		name     += 4;
-	}
-
-	if (name_len != 0) {
-		name_word = 0;
-
-		if (2 <= name_len) {
-			/* name_word = (uint32_t)*((uint16_t *)name); */
-			bytes[0] = name[0];
-			bytes[1] = name[1];
-			name_word |= (bytes[1] << 8) | bytes[0];
-			name_len -= 2;
-			name     += 2;
-		}
-
-		if (name_len == 1)
-			name_word |= ((uint32_t)*name) << 16;
-
-		PLATFORM_HASH32(hash_state, name_word);
-	}
-
-	hash_value = PLATFORM_HASH32_FINISH(hash_state, name_kind);
-	return hash_value;
+	return odp_hash_crc32c(name, strlen(name), name_kind);
 }
 
 static uint32_t linked_list_len(name_tbl_entry_t *name_tbl_entry)