On 12/08/2015 10:50, Savolainen, Petri (Nokia - FI/Espoo) wrote:
Bill,
hash_tbl_entry is now uintptr_t, which is 64 bits on a 64 bit build and 32 bits
on a 32 bit build. These shifts and all other lines that expect that
hash_tbl_entry (pointer size) is 64 bits should be updated also.
-Petri
Yes, that is what I said before. For now we can go with simple using 64
bit hash entries for 32 bit case also.
Where 6 upper bits will be for defining first or secondary hash and it's
size. And all lower 32 for address pointer.
I.e. 26 bits will be lost.
#define PRIMARY_HASH_TBL_SIZE (16 * 1024)
26 * PRIMARY_HASH_TBL_SIZE = 425984 = 53248 bytes = 13 pages;
#define SECONDARY_HASH_TBL_SIZE 128
26 * SECONDARY_HASH_TBL_SIZE = 3328 = 416 bytes
So we lost for 32 bits some memory but code has to work in the same way
as for 64 bits.
After that if memory efficient fix will be needed for somebody than he
can fix it.
Or we will reuse some odp hash table api instead of odp_name_table.c.
So for now I would do quick fix with some comment that there is unused
memory for 32 bit case.
--- a/platform/linux-generic/odp_name_table.c
+++ b/platform/linux-generic/odp_name_table.c
@@ -93,7 +93,7 @@ typedef struct {
* NOT zero then this values points to a (linked list of)
name_table_entry_t
* records AND the bottom 6 bits are the length of this list.
*/
-typedef uintptr_t hash_tbl_entry_t;
+typedef uint64_t hash_tbl_entry_t;
BR,
Maxim.
-----Original Message-----
From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
Maxim Uvarov
Sent: Monday, December 07, 2015 5:01 PM
To: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: tm: use
odp_hash_crc32c() api to avoid arch issues
shift still is not fixed:
odp_name_table.c:186:4: error: right shift count >= width of type [-
Werror]
if ((hash_tbl_entry >> 48) == 0x7FFF)
^
odp_name_table.c:188:4: error: right shift count >= width of type [-
Werror]
else if ((hash_tbl_entry >> 48) == 0)
On 12/07/2015 14:09, 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.fischo...@linaro.org>
---
platform/linux-generic/odp_name_table.c | 184 +-----------------------
--------
1 file changed, 2 insertions(+), 182 deletions(-)
diff --git a/platform/linux-generic/odp_name_table.c b/platform/linux-
generic/odp_name_table.c
index 10a760e..b4a9081 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
@@ -227,7 +93,7 @@ typedef struct {
* NOT zero then this values points to a (linked list of)
name_table_entry_t
* records AND the bottom 6 bits are the length of this list.
*/
-typedef uint64_t hash_tbl_entry_t;
+typedef uintptr_t hash_tbl_entry_t;
typedef struct {
hash_tbl_entry_t hash_entries[SECONDARY_HASH_TBL_SIZE];
@@ -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
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
