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

[Neil Horman <nhorman@tuxdriver.com>]

On Fri, Nov 14, 2014 at 10:43:39PM +0600, Yerden Zhumabekov wrote:
> 
> 14.11.2014 19:53, Neil Horman пишет:
> > On Fri, Nov 14, 2014 at 05:57:51PM +0600, Yerden Zhumabekov wrote:
> >> 14.11.2014 17:33, Neil Horman пишет:
> >>> 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.
> >> I see, then we have a problem here :)
> >>
> >> Actually, that was one of my concerns when developing these patches. I
> >> looked through the source code of libs and examples and I saw the
> >> '#ifdef..#include..#endif'-like appoach while selecting hash function
> >> was common. So I organized patches to minimize the impact on API and not
> >> to contradict this approach.
> >>
> > Thats a reasonable approach, but I really hate the idea of continuing this need
> > to select cpu features at compile time if its not nececcesary.
> >
> >> If we prefer to change this approach then, I guess, we need to introduce
> >> broader changes to rte_hash library and change other code which uses it.
> >> If that's what's needed, then it'll take some time for me to rework
> >> these patches.
> >>
> > Well, its possible you'll get lucky.  crc is such a common operation, its
> > entirely possible that the gcc intrinsic emits software based crc computation if
> > the SSE4.2 instructions aren't enabled.  I recommend modifying the test_hash_crc
> > function to use rte_hash_crc with SSE4.2 disabled, and see if you get a crash.
> > If you don't examine the disassembly of your new function and confirm that
> > something reasonable that doesn't use SSE4.2 is emitted.  If thats the case,
> > your patch is fine, and we can focus on how to change the ifdefs in the existing
> > code, as use of the rte_hash_crc functions should be safe.
> >
> 
> Unfortunately, it seems not to be the case. Trying to force compiling a
> test program with _mm_crc32_u32 intrinsic on computer with no SSE4.2
> support leads to "Illegal instruction error". So it looks like GCC does
> not fall back to crc32 software implementation.
> 
Ok, but crc32 is pretty easy to implement in software.  Just appropriate the
calculate_crc32c function from the BSD or Linux kernels and if
(unlikely(!support_sse42)) calculate_crc32 operation.

Neil

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