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

[Yerden Zhumabekov <e_zhumabekov-8EHiFRVJVgQ@public.gmane.org>]

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.

-- 
Sincerely,

Yerden Zhumabekov
State Technical Service
Astana, KZ