[EXT] Re: [PATCH 2/2] config/arm: disable SVE for cn10k

[fengchengwen]

On 2022/5/6 21:17, Rahul Bhansali wrote:
> 
> 
>> -----Original Message-----
>> From: Ruifeng Wang <Ruifeng.Wang at arm.com>
>> Sent: Friday, May 6, 2022 12:53 PM
>> To: fengchengwen <fengchengwen at huawei.com>; Rahul Bhansali
>> <rbhansali at marvell.com>; dev at dpdk.org; Jan Viktorin
>> <viktorin at rehivetech.com>; Bruce Richardson <bruce.richardson at intel.com>
>> Cc: Jerin Jacob Kollanukkaran <jerinj at marvell.com>; nd <nd at arm.com>
>> Subject: RE: [EXT] Re: [PATCH 2/2] config/arm: disable SVE for cn10k
>>
>>> -----Original Message-----
>>> From: fengchengwen <fengchengwen at huawei.com>
>>> Sent: Friday, May 6, 2022 2:36 PM
>>> To: Rahul Bhansali <rbhansali at marvell.com>; dev at dpdk.org; Ruifeng Wang
>>> <Ruifeng.Wang at arm.com>; Jan Viktorin <viktorin at rehivetech.com>; Bruce
>>> Richardson <bruce.richardson at intel.com>
>>> Cc: jerinj at marvell.com
>>> Subject: Re: [EXT] Re: [PATCH 2/2] config/arm: disable SVE for cn10k
>>>
>>> On 2022/5/6 12:54, Rahul Bhansali wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: fengchengwen <fengchengwen at huawei.com>
>>>>> Sent: Friday, May 6, 2022 8:00 AM
>>>>> To: Rahul Bhansali <rbhansali at marvell.com>; dev at dpdk.org; Ruifeng
>>>>> Wang <ruifeng.wang at arm.com>; Jan Viktorin
>>> <viktorin at rehivetech.com>;
>>>>> Bruce Richardson <bruce.richardson at intel.com>
>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj at marvell.com>
>>>>> Subject: [EXT] Re: [PATCH 2/2] config/arm: disable SVE for cn10k
>>>>>
>>>>> External Email
>>>>>
>>>>> -------------------------------------------------------------------
>>>>> --
>>>>> - On 2022/5/5 22:27, Rahul Bhansali wrote:
>>>>>> This disable the SVE flag for cn10k.
>>>>>>
>>>>>> Performance impact:-
>>>>>> With l3fwd example, lpm lookup performance increased by ~21% if
>>> Neon
>>>>>> is used instead of SVE.
>>>>>>
>>>>>> Signed-off-by: Rahul Bhansali <rbhansali at marvell.com>
>>>>>> ---
>>>>>>  config/arm/meson.build | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
>>>>>> dafb342cc6..39b7a1270c 100644
>>>>>> --- a/config/arm/meson.build
>>>>>> +++ b/config/arm/meson.build
>>>>>> @@ -281,7 +281,8 @@ soc_cn10k = {
>>>>>>      ],
>>>>>>      'part_number': '0xd49',
>>>>>>      'extra_march_features': ['crypto'],
>>>>>> -    'numa': false
>>>>>> +    'numa': false,
>>>>>> +    'sve': false
>>>>>
>>>>> Suggest remove sve2 flag:
>>>>>     '0xd49': {
>>>>>         'march': 'armv8.5-a',
>>>>>         'march_features': ['sve2'],          ---remove 'sve2'
>>>>>         'flags': [
>>>>>             ['RTE_MACHINE', '"neoverse-n2"'],
>>>>>             ['RTE_ARM_FEATURE_ATOMICS', true],
>>>>>             ['RTE_MAX_LCORE', 64],
>>>>>             ['RTE_MAX_NUMA_NODES', 1]
>>>>>         ]
>>>>>     }
>>>>>
>>>> If I remove here, then this will also change for " Arm Neoverse N2
>>>> soc_n2",
>>> because part_number is same, Right ?
>>>> Because of this reason, I thought to have separate flag instead of
>>>> updating
>>> march_features.
>>>
>>> This new add flag only impact hand-writen sve code, but
>>> auto-vectorization is also enabled when sve is enabled at march_features.
>> Agree.
>>
>>> Maybe NEON-based automated vector code performs better than SVE-
>>> based.
>>>
>>> I think it's OK to add separate flag in soc_xxx struct, but suggest it
>>> also impact auto-vectorization.
>> I would suggest the flag to control only RTE_HAS_SVE_ACLE, i.e. hand written
>> code using SVE C language intrinsics.
>> For auto-vectorization, I think it is compilers duty to vectorize in the most
>> performant way, use whatever resource hardware provided.
>>
> 
> I agree to the point of auto-vectorization to let it be if supported and control the RTE_HAS_SVE_ACLE for hand-written SVE C code.

Agree with the point: the flag to control only RTE_HAS_SVE_ACLE
Maybe sve_acle is more appropriate for the new flag.

> 
>>>
>>> So for one soc which test or optimize well on sve, it could turn the flag to true.
>>>
>>>>
>>>>>>  }
>>>>>>
>>>>>>  soc_dpaa = {
>>>>>>
>>>>
>