Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 5/18/2021 4:15 PM, Bruce Richardson wrote:
> On Tue, May 18, 2021 at 03:40:18PM +0100, Ferruh Yigit wrote:
>> On 5/14/2021 10:53 AM, Chengwen Feng wrote:
>>> Currently, the SVE code is compiled only when -march supports SVE
>>> (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this
>>> approach.
>>>
>>> The solution:
>>> a. If the minimum instruction set support SVE then compiles it.
>>> b. Else if the compiler support SVE then compiles it.
>>> c. Otherwise don't compile it.
>>>
>>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
>>>
>>
>> Hi Chengwen,
>>
>> As far as I understand from above problem statement, you want to produce a
>> binary that can run in two different platforms, one supports only NEON
>> instructions, other supports NEON + SVE.
>>
>> For this driver should be compiled in a way to support min instruction set,
>> which is NEON.
>>
>> There are two build items,
>>
>> 1) hns3_rxtx_vec_sve.c
>> 2) rest of the library
>>
>> There is already runtime checks to select Rx/Tx functions, so it is safe to
>> build (1) as long as compiler supports. If the platform doesn't support SVE, the
>> SVE path won't be selected during runtime.
>>
>> For (2), it should be build to support NEON only, if it is compiled to support
>> SVE, it won't run on the platform that only supports NEON.
>>
>> So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is build with SVE
>> support, won't this cause a problem on the NEON platform?
>>
> In that case, the rest of DPDK is being build with SVE so having one driver
> neon-only doesn't really make sense.
> 
> Overall, the patch looks to mirror what we do for AVX2/AVX512 support in
> the Intel drivers, so looks ok to me.
> 

I agree there is no point to make driver specific change if whole DPDK build
with SVE, and that seems controlled by user as Honnappa clarified.
So I will proceed with patch, thanks.