Re: [dpdk-dev] [PATCH 0/2] rewritten rte_hash_crc() call

[Neil Horman <nhorman@tuxdriver.com>]

On Fri, Nov 14, 2014 at 01:15:12PM +0600, Yerden Zhumabekov wrote:
> 14.11.2014 6:52, Neil Horman пишет:
> > On Thu, Nov 13, 2014 at 06:33:14PM +0100, Thomas Monjalon wrote:
> >> Any comment on these patches?
> >>
> >> 2014-09-03 12:05, Yerden Zhumabekov:
> >>> As SSE4.2 provides CRC32 instructions with either 32 and 64 bit operands,
> >>> new rte_hash_crc_8byte() call assisted with _mm_crc32_u64 intrinsic may be
> >>> useful.
> >>>
> >>> ... <skipped> ...
> >>
> > Yeah, sorry I didn't speak up earlier.  I meant to ask if the __mm_crc_u64
> > intrinsic will emit software emulated versions of the sse4.2 instruction in the
> > event that you build with a config that doesn't enable sse4.2?  If not, then
> > NAK, since this will break on the default build.  In that event you'll have to
> > modify the new function to do a runtime cpu flags check to either just use the
> > instruction inlined with some asm, or emulate it in software.
> 
> Hello,
> 
> A quick grep on dpdk source shows that rte_hash_crc() is used in
> librte_hash in following context:
> 
> In rte_hash.c:
> /* Hash function used if none is specified */
> #ifdef RTE_MACHINE_CPUFLAG_SSE4_2
> #include <rte_hash_crc.h>
> #define DEFAULT_HASH_FUNC       rte_hash_crc
> #else
> #include <rte_jhash.h>
> #define DEFAULT_HASH_FUNC       rte_jhash
> #endif
> 
> In rte_fbk_hash.h
> #ifdef RTE_MACHINE_CPUFLAG_SSE4_2
> #include <rte_hash_crc.h>
> /** Default four-byte key hash function if none is specified. */
> #define RTE_FBK_HASH_FUNC_DEFAULT·······rte_hash_crc_4byte
> #else
> #include <rte_jhash.h>
> #define RTE_FBK_HASH_FUNC_DEFAULT·······rte_jhash_1word
> #endif
> #endif
> 
> 
> I guess it covers the cpu flags check you're talking about.
> 

Not really.  That covers the case of applications selecting the hash function
using the DEFUALT_HASH_FUNC macro, but doesn't nothing for applications using
the function directly.  Test_hash_perf is an example  of this, and ostensibly
because of the behavior without SSE4.2 it defines these huge test tables twice
based on the availability of SSE4.2.  It would be better if we could allow
applications to use rte_hash_crc regardless, and make the code it uses at run
time configurable.
Neil

> -- 
> Sincerely,
> 
> Yerden Zhumabekov
> State Technical Service
> Astana, KZ
> 
> 
>