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

[De Lara Guarch, Pablo]

Hi Tony,

> -----Original Message-----
> From: Tony Lu [mailto:zlu at ezchip.com]
> Sent: Friday, July 17, 2015 8:58 AM
> To: De Lara Guarch, Pablo; dev at 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 at intel.com]
> >Sent: Friday, July 17, 2015 3:35 PM
> >To: Tony Lu; dev at 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 at ezchip.com]
> >> Sent: Friday, July 17, 2015 4:35 AM
> >> To: De Lara Guarch, Pablo; dev at 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 at intel.com]
> >> >Sent: Friday, July 17, 2015 4:42 AM
> >> >To: Tony Lu; dev at 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 at ezchip.com]
> >> >> Sent: Thursday, July 16, 2015 10:40 AM
> >> >> To: De Lara Guarch, Pablo; dev at 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
> >>
>