Re: [dpdk-dev] [PATCH v4 3/5] hash: add fallback to software CRC32 implementation

["Wang, Shawn" <xingbow@amazon.com>]

I have a general question about using CPUID to detect supported instruction set.
What if we are compiling the software with some old hardware which does not support SSE4.2, but run it on new hardware which does support SSE4.2. Is there still a static way to force the compiler to turn on the SSE4.2 support? 
I guess for SSE4.2, most of the CPU has support for it now. But for AVX2, this might not be the case.
________________________________________
From: dev [dev-bounces@dpdk.org] on behalf of Yerden Zhumabekov [e_zhumabekov@sts.kz]
Sent: Tuesday, November 18, 2014 9:13 AM
To: Neil Horman; Richardson, Bruce; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v4 3/5] hash: add fallback to software CRC32     implementation

18.11.2014 22:00, Neil Horman пишет:
> On Tue, Nov 18, 2014 at 09:06:35PM +0600, Yerden Zhumabekov wrote:
>> 18.11.2014 20:41, Neil Horman пишет:
>>> On Tue, Nov 18, 2014 at 08:03:40PM +0600, Yerden Zhumabekov wrote:
>>>>  /**
>>>>   * Use single crc32 instruction to perform a hash on a 4 byte value.
>>>> + * Fall back to software crc32 implementation in case SSE4.2 is
>>>> + * not supported
>>>>   *
>>>>   * @param data
>>>>   *   Data to perform hash on.
>>>> @@ -376,11 +413,18 @@ crc32c_2words(uint64_t data, uint32_t init_val)
>>>>  static inline uint32_t
>>>>  rte_hash_crc_4byte(uint32_t data, uint32_t init_val)
>>>>  {
>>>> -  return _mm_crc32_u32(init_val, data);
>>>> +#ifdef RTE_MACHINE_CPUFLAG_SSE4_2
>>>> +  if (likely(crc32_alg == CRC32_SSE42))
>>>> +          return _mm_crc32_u32(init_val, data);
>>>> +#endif
>>> you don't really need these ifdefs here anymore given that you have a
>>> constructor to do the algorithm selection.  In fact you need to remove them, in
>>> the event you build on a system that doesn't support SSE42, but run on a system
>>> that does.
>> Originally, I thought so as well. I wrote the code without these ifdefs,
>> but it didn't compile on my machine which doesn't support SSE4.2. Error
>> was triggered by nmmintrin.h which has a check for respective GCC
>> extension. So I think these ifdefs are indeed required.
>>
> You need to edit the makefile so that the compiler gets passed the option
> -msse42.  That way it will know to emit sse42 instructions. It will also allow
> you to remove the ifdef from the include file

In this case, I guess there are two options:
1) modify all makefiles which use librte_hash
2) move all function bodies from rte_hash_crc.h to separate module,
leaving prototype definitions there only.

Everybody's up for the second option? :)

--
Sincerely,

Yerden Zhumabekov
State Technical Service
Astana, KZ