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

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

15.11.2014 0:41, Neil Horman пишет:
> On Fri, Nov 14, 2014 at 10:43:39PM +0600, Yerden Zhumabekov wrote:
>> 14.11.2014 19:53, Neil Horman пишет:
>>>
>>> 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.
>

I've almost reworked patches, but there's one more issue I was wondering
about.

If we use a flag (say, 'sse42_flag ') to determine code path, where
should it be defined?
Should it be some sort of rte_hash_crc_init() call in the init stage of
application?

Alternatively, I could have implemented it like this:


static uint8_t sse42_flag = FLAG_UNKNOWN;
....
rte_hash_crc_4byte(uint32_t data, uint32_t init_val)
{
    if (unlikely(sse42_flag == FLAG_UNKNOWN))
        sse42_flag = rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_2) ?
FLAG_SUPPORTED : FLAG_NOTSUPPORTED;

    if (likely(sse42_flag == FLAG_SUPPORTED))
        return _mm_crc32_u32(init_val, data);
    .....
}



-- 
Sincerely,

Yerden Zhumabekov
State Technical Service
Astana, KZ