Re: [dpdk-dev] [PATCH 1/2] config/arm: add non-SVE march for soc kunpeng930

[fengchengwen <fengchengwen@huawei.com>]

On 2021/5/13 7:00, Honnappa Nagarahalli wrote:
> <snip>
> 
>>
>> On Wed, May 12, 2021 at 2:01 PM Chengwen Feng
>> <fengchengwen@huawei.com> wrote:
>>>
>>> Currently, the soc_kunpeng930 declares '-march=armv8.2-a+crypto+sve',
>>> but some compiler doesn't recognize the march because it doesn't
>>> support sve.
>>
>> Agree with the problem statement.
>>
>> + @Juraj Linkeš
>>
>>>
>>> This patch adds '-march=armv8.2-a+crypto' before
>>> '-march=armv8.2-a+crypto+sve' so that:
>>> 1. If compiler doesn't support '-march=armv8.2-a+crypto+sve', then it
>>> will fallback supports 'armv8.2-a+crypto'.
>>> 2. If compiler supports '-march=armv8.2-a+crypto+sve', then it will
>>> compile SVE-related code.
>>>
>>> Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")
>>>
>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>> ---
>>>  config/arm/meson.build | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
>>> 3f34ec9..fe6c29b 100644
>>> --- a/config/arm/meson.build
>>> +++ b/config/arm/meson.build
>>> @@ -158,7 +158,7 @@ implementer_hisilicon = {
>>>              ]
>>>          },
>>>          '0xd02': {
>>> -            'machine_args': ['-march=armv8.2-a+crypto+sve'],
>>> +            'machine_args': ['-march=armv8.2-a+crypto',
>>> + '-march=armv8.2-a+crypto+sve'],
>>
>> I think, this problem not specific to Kunpeng. I think, in order to have a generic
>> solution, I think, we need to express something like [[armv8.2-a], [crypto],
>> [sve]] or so in the code and any infra code can generate a working
>> combination to avoid duplicating the combination manually in code and have
>> some generic solution for all the platforms.
> I think this approach will be too generic. We have to start with v8.0 (as the compiler might not support v8.2-a) and list every feature and check if all those features are supported in the compiler. It might be required that some features might not perform well in certain applications which requires for disabling the features.
> 
> I think tweaks like this should be left to the end user.
> 

Agree

Fixed in v2 (separate -march='armv8.2-a+crypto+sve' to two tuples, namely: march_base and march_feature), thanks

>>
>> i.e
>> patch 1: Enable the infrastructure for auto probe based on compiler support.
>> patch 2: Fix existing machines including the Kunpeng.
>>
>>
>>>              'flags': [
>>>                  ['RTE_MACHINE', '"Kunpeng 930"'],
>>>                  ['RTE_ARM_FEATURE_ATOMICS', true],
>>> --
>>> 2.8.1
>>>