Re: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, March 30, 2016 4:50 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>; Zhang,
> Roy Fan <roy.fan.zhang@intel.com>; Hunt, David <david.hunt@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> x86_64 without SSE4.2
> 
> 2016-03-30 14:15, Dumitrescu, Cristian:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2016-03-30 13:57, Dumitrescu, Cristian:
> > > > I think the correct fix is:
> > > > #if defined(__x86_64__) &&
> (defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> > > || defined(RTE_MACHINE_CPUFLAG_CRC32))
> > > >
> > > > We'll test it and send a patch asap.
> > >
> > > I had prepared this patch. Please be inspired:
> > >
> > >     examples/ip_pipeline: fix SSE4.2 optimization branch
> > >
> > >     The branch was disabled because of a typo in the SSE4.2 flag.
> > >     Change also the x86_64 flag to use a DPDK one.
> > >
> > >     Fixes: 28377375c6c0 ("examples/ip_pipeline: fix build for x86_64
> without
> > > SSE4.2")
> > >
> > > -#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> > > +#if defined(RTE_ARCH_X86_64) &&
> > > defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> >
> > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> 
> I thought you wanted to send a patch with another CPUFLAG (CRC32)?

The extra flag is good, but not absolutely required, as SSE4.2 implies support for CRC32 instruction.

The CRC32 flag might be useful when a CPU architecture other than Intel supports the CRC32 instruction, but I am not sure whether such CPU architecture exists. Anyway, the SSE4.2 || CRC32 pattern is already present in several DPDK files, so I was looking to observe it as well.

I thought you considered the CRC32 flag to be redundant and decided to remove it on purpose.

I am OK if you want to go ahead with your patch right now, otherwise we can send a patch tomorrow?

Thanks,
Cristian