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

Message ID 1620808126-18876-2-git-send-email-fengchengwen@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series bugfix for Kunpeng930 SVE compile |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chengwen Feng May 12, 2021, 8:28 a.m. UTC
  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.

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(-)
  

Comments

Jerin Jacob May 12, 2021, 8:44 a.m. UTC | #1
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.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
>
  
Honnappa Nagarahalli May 12, 2021, 11 p.m. UTC | #2
<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.

> 
> 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
> >
  
Chengwen Feng May 13, 2021, 4:49 a.m. UTC | #3
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
>>>
  

Patch

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'],
             'flags': [
                 ['RTE_MACHINE', '"Kunpeng 930"'],
                 ['RTE_ARM_FEATURE_ATOMICS', true],