Re: [dpdk-dev] [PATCH v7 1/7] hash: replace existing hash library with cuckoo hash implementation

[Tony Lu <zlu@ezchip.com>]

Hi, Pablo

The patch "hash: fix compilation for non-x86 systems " works for no-X86
arches.
Thanks for your quick fix.

>-----Original Message-----
>From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
>Sent: Friday, July 17, 2015 5:06 PM
>To: Tony Lu; dev@dpdk.org
>Subject: RE: [dpdk-dev] [PATCH v7 1/7] hash: replace existing hash library
with
>cuckoo hash implementation
>
>Hi Tony,
>
>> -----Original Message-----
>> From: Tony Lu [mailto:zlu@ezchip.com]
>> Sent: Friday, July 17, 2015 8:58 AM
>> To: De Lara Guarch, Pablo; dev@dpdk.org
>> Subject: RE: [dpdk-dev] [PATCH v7 1/7] hash: replace existing hash
>> library with cuckoo hash implementation
>>
>> >-----Original Message-----
>> >From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
>> >Sent: Friday, July 17, 2015 3:35 PM
>> >To: Tony Lu; dev@dpdk.org
>> >Subject: RE: [dpdk-dev] [PATCH v7 1/7] hash: replace existing hash
>> >library
>> with
>> >cuckoo hash implementation
>> >
>> >
>> >
>> >> -----Original Message-----
>> >> From: Tony Lu [mailto:zlu@ezchip.com]
>> >> Sent: Friday, July 17, 2015 4:35 AM
>> >> To: De Lara Guarch, Pablo; dev@dpdk.org
>> >> Subject: RE: [dpdk-dev] [PATCH v7 1/7] hash: replace existing hash
>> >> library with cuckoo hash implementation
>> >>
>> >> Hi, Pablo
>> >>
>> >> >-----Original Message-----
>> >> >From: De Lara Guarch, Pablo
>> >> >[mailto:pablo.de.lara.guarch@intel.com]
>> >> >Sent: Friday, July 17, 2015 4:42 AM
>> >> >To: Tony Lu; dev@dpdk.org
>> >> >Subject: RE: [dpdk-dev] [PATCH v7 1/7] hash: replace existing hash
>> >> >library
>> >> with
>> >> >cuckoo hash implementation
>> >> >
>> >> >Hi Tony,
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Tony Lu [mailto:zlu@ezchip.com]
>> >> >> Sent: Thursday, July 16, 2015 10:40 AM
>> >> >> To: De Lara Guarch, Pablo; dev@dpdk.org
>> >> >> Subject: RE: [dpdk-dev] [PATCH v7 1/7] hash: replace existing
>> >> >> hash library with cuckoo hash implementation
>> >> >>
>> >> >> >diff --git a/lib/librte_hash/rte_cuckoo_hash.c
>> >> >> b/lib/librte_hash/rte_cuckoo_hash.c
>> >> >> >new file mode 100644
>> >> >> >index 0000000..50e3acd
>> >> >> >--- /dev/null
>> >> >> >+++ b/lib/librte_hash/rte_cuckoo_hash.c
>> >> >> >@@ -0,0 +1,1027 @@
>> >> >> ...
>> >> >> >+
>> >> >> >+/* Functions to compare multiple of 16 byte keys (up to 128
>> >> >> >+bytes) */ static int rte_hash_k16_cmp_eq(const void *key1,
>> >> >> >+const void *key2, size_t key_len
>> >> >> >__rte_unused)
>> >> >> >+{
>> >> >> >+	const __m128i k1 = _mm_loadu_si128((const __m128i *)
>> key1);
>> >> >> >+	const __m128i k2 = _mm_loadu_si128((const __m128i *)
>> key2);
>> >> >> >+	const __m128i x = _mm_xor_si128(k1, k2);
>> >> >> >+
>> >> >> >+	return !_mm_test_all_zeros(x, x); }
>> >> >> ...
>> >> >>
>> >> >> When compiling the latest dev DPDK for non-x86 arch, it fails on
>> >> >> the above code, as the SSE is x86 specific defined in
>> >> >> <emmintrin.h>.  Is it possible to replace this function with
>> >> >> platform
>> >independent code?
>> >> >
>> >> >Thanks for spotting this. I just sent a patch that should fix the
>> problem.
>> >> >Can you check if it works?
>> >>
>> >> Thanks for your quick response, but __m128i and all the _mm_
>> >> related functions are X86 specific defined in <emmintrin.h>.  This
>> >> header file is only available in X86 compiler library, but no-X86
>> >> archs do not have this file.  So if we can replace all the X86
>> >> specific code in the above function, that would be great.
>> >>
>> >With the patch that I sent, if you are compiling for a non-x86 arch,
>> >you
>> should
>> >not have any problem, since all that code will only be used if using
>> >x86
>> arch.
>> >Have you tried compiling DPDK with the patch?
>>
>> Yes, I have built the DPDK with your patch, and got the following errors.
>> This is
>> because there are no __m128i, _mm_loadu_si128(), _mm_cmpeq_epi32() and
>> _mm_movemask_epi8() on no-X86 arches.
>>
>> == Build lib/librte_hash
>>   CC rte_cuckoo_hash.o
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c: In function
>> 'rte_hash_k16_cmp_eq':
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1126: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'k1'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1126: error:
'k1'
>> undeclared (first use in this function)
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1126: error:
>> (Each undeclared identifier is reported only once
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1126: error:
>> for each function it appears in.)
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1126: warning:
>> implicit declaration of function '_mm_loadu_si128'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1126: warning:
>> nested extern declaration of '_mm_loadu_si128'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1126: error:
>> expected ')' before '__m128i'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1126: warning:
>> type defaults to 'int' in declaration of 'type name'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1126: warning:
>> cast from pointer to integer of different size
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1127: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'k2'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1127: error:
'k2'
>> undeclared (first use in this function)
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1127: error:
>> expected ')' before '__m128i'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1127: warning:
>> type defaults to 'int' in declaration of 'type name'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1127: warning:
>> cast from pointer to integer of different size
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1133: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'x'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1133: error:
'x'
>> undeclared (first use in this function)
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1133: warning:
>> implicit declaration of function '_mm_cmpeq_epi32'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1133: warning:
>> nested extern declaration of '_mm_cmpeq_epi32'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1135: warning:
>> implicit declaration of function '_mm_movemask_epi8'
>> /u/zlu.bjg/git/dpdk.org/lib/librte_hash/rte_cuckoo_hash.c:1135: warning:
>> nested extern declaration of '_mm_movemask_epi8'
>> make[3]: *** [rte_cuckoo_hash.o] Error 1
>> make[2]: *** [librte_hash] Error 2
>> make[1]: *** [lib] Error 2
>> make: *** [all] Error 2
>
>Looking at the snippet, I would say the patch has not been applied (did you
apply
>the patch hash: "fix compilation for non-x86 systems"?), because looking at
the
>lines where it is failing, they are the old lines in rte_cuckoo_hash.c,
which have
>been moved to another file (and should not be included for you).
>Pablo
>>
>> Thanks
>> -Tony
>>
>> >Pablo
>> >
>> >> Thanks
>> >> -Tony
>> >>
>> >>
>> >> >Thanks,
>> >> >Pablo
>> >> >>
>> >> >> Thanks
>> >> >> -Zhigang Lu
>> >>
>>