diff mbox series

[v4,1/2] config/arm: select most suitable -march for kunpeng soc

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

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

fengchengwen May 13, 2021, 1:08 p.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.

To solve this bug we use the following scheme:
1. Define 'march_base' tuple which defines support march, it should
arrange from lower to higher.
e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a']
2. Define 'march_feature' tuple which defines support feature.
e.g. 'march_feature' : ['crypto', 'sve']
3. Select the most suitable march+feature combination based on
'march_base' and 'march_feature' tuples.
4. Use the selected march+feature combination as the default
machine_args.

Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 config/arm/meson.build | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

Comments

Jerin Jacob May 13, 2021, 3:24 p.m. UTC | #1
On Thu, May 13, 2021 at 6:41 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.
>
> To solve this bug we use the following scheme:
> 1. Define 'march_base' tuple which defines support march, it should
> arrange from lower to higher.
> e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a']
> 2. Define 'march_feature' tuple which defines support feature.
> e.g. 'march_feature' : ['crypto', 'sve']
> 3. Select the most suitable march+feature combination based on
> 'march_base' and 'march_feature' tuples.
> 4. Use the selected march+feature combination as the default
> machine_args.
>
> Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  config/arm/meson.build | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 3f34ec9..8551a80 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -149,7 +149,9 @@ implementer_hisilicon = {
>      ],
>      'part_number_config': {
>          '0xd01': {
> -            'machine_args': ['-march=armv8.2-a+crypto', '-mtune=tsv110'],
> +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],

Another target has the same issue. Could you fix that all together as
it is generic problem in the existing infrastructure.


> +            'march_feature' : ['crypto'],
> +            'machine_args': ['-mtune=tsv110'],
>              'flags': [
>                  ['RTE_MACHINE', '"Kunpeng 920"'],
>                  ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -158,7 +160,9 @@ implementer_hisilicon = {
>              ]
>          },
>          '0xd02': {
> -            'machine_args': ['-march=armv8.2-a+crypto+sve'],
> +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> +            'march_feature' : ['crypto', 'sve'],
> +            'machine_args': [],
>              'flags': [
>                  ['RTE_MACHINE', '"Kunpeng 930"'],
>                  ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -449,8 +453,27 @@ else
>      # add/overwrite flags in the proper order
>      dpdk_flags = flags_common + implementer_config['flags'] + part_number_config.get('flags', []) + soc_flags
>
> +    # select the most suitable march+feature combination
> +    machine_march = []
> +    if part_number_config.has_key('march_base')
> +        foreach arch: part_number_config['march_base']
> +            if cc.has_argument(arch)
> +                machine_march = arch # Set the higher supported arch as possible
> +            endif
> +        endforeach
> +    endif
> +    if part_number_config.has_key('march_feature')
> +        foreach feature: part_number_config['march_feature']
> +            tmp_machine_march = machine_march + '+' + feature
> +            if cc.has_argument(tmp_machine_march)
> +                machine_march = tmp_machine_march # Set the more supported feature as possible
> +            endif
> +        endforeach
> +    endif
> +
>      # apply supported machine args
>      machine_args = [] # Clear previous machine args
> +    machine_args += machine_march
>      foreach flag: part_number_config['machine_args']
>          if cc.has_argument(flag)
>              machine_args += flag
> --
> 2.8.1
>
Honnappa Nagarahalli May 13, 2021, 11:12 p.m. UTC | #2
<snip>
> 
> On Thu, May 13, 2021 at 6:41 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.
> >
> > To solve this bug we use the following scheme:
> > 1. Define 'march_base' tuple which defines support march, it should
> > arrange from lower to higher.
> > e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'] 2. Define
> > 'march_feature' tuple which defines support feature.
> > e.g. 'march_feature' : ['crypto', 'sve'] 3. Select the most suitable
> > march+feature combination based on 'march_base' and 'march_feature'
> > tuples.
> > 4. Use the selected march+feature combination as the default
> > machine_args.
> >
> > Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")
> >
> > Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > ---
> >  config/arm/meson.build | 27 +++++++++++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > 3f34ec9..8551a80 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -149,7 +149,9 @@ implementer_hisilicon = {
> >      ],
> >      'part_number_config': {
> >          '0xd01': {
> > -            'machine_args': ['-march=armv8.2-a+crypto', '-mtune=tsv110'],
> > +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
If the compiler supports armv8.1-a, you need to choose armv8.1-a.

> 
> Another target has the same issue. Could you fix that all together as it is
> generic problem in the existing infrastructure.
I think this needs to be more generic solution. IMO, the requirement is as follows:

1) We need to pick the most closest march_base supported by the compiler. For ex: If the SoC support v8.2 and the compiler supports v8.1, we need to pick v8.1
2) SoCs are allowed to support a base march version + hand picked features from the next 1 base marchs. i.e. armv8.x compliant implementation can include any features from armv8.(x + 1). Please refer to  Arm-ARM section A2 for more details. So, it is possible that the compiler supports a base march and a bunch of optional features from the next version. We need to test all the features allowed by the architecture and pick the ones that are supported in the compiler.

> 
> 
> > +            'march_feature' : ['crypto'],
> > +            'machine_args': ['-mtune=tsv110'],
> >              'flags': [
> >                  ['RTE_MACHINE', '"Kunpeng 920"'],
> >                  ['RTE_ARM_FEATURE_ATOMICS', true], @@ -158,7 +160,9
> > @@ implementer_hisilicon = {
> >              ]
> >          },
> >          '0xd02': {
> > -            'machine_args': ['-march=armv8.2-a+crypto+sve'],
> > +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> > +            'march_feature' : ['crypto', 'sve'],
> > +            'machine_args': [],
> >              'flags': [
> >                  ['RTE_MACHINE', '"Kunpeng 930"'],
> >                  ['RTE_ARM_FEATURE_ATOMICS', true], @@ -449,8 +453,27
> > @@ else
> >      # add/overwrite flags in the proper order
> >      dpdk_flags = flags_common + implementer_config['flags'] +
> > part_number_config.get('flags', []) + soc_flags
> >
> > +    # select the most suitable march+feature combination
> > +    machine_march = []
> > +    if part_number_config.has_key('march_base')
> > +        foreach arch: part_number_config['march_base']
> > +            if cc.has_argument(arch)
> > +                machine_march = arch # Set the higher supported arch as
> possible
> > +            endif
> > +        endforeach
> > +    endif
> > +    if part_number_config.has_key('march_feature')
> > +        foreach feature: part_number_config['march_feature']
> > +            tmp_machine_march = machine_march + '+' + feature
> > +            if cc.has_argument(tmp_machine_march)
> > +                machine_march = tmp_machine_march # Set the more supported
> feature as possible
> > +            endif
> > +        endforeach
> > +    endif
> > +
> >      # apply supported machine args
> >      machine_args = [] # Clear previous machine args
> > +    machine_args += machine_march
> >      foreach flag: part_number_config['machine_args']
> >          if cc.has_argument(flag)
> >              machine_args += flag
> > --
> > 2.8.1
> >
fengchengwen May 14, 2021, 10:23 a.m. UTC | #3
On 2021/5/14 7:12, Honnappa Nagarahalli wrote:
> <snip>
>>
>> On Thu, May 13, 2021 at 6:41 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.
>>>
>>> To solve this bug we use the following scheme:
>>> 1. Define 'march_base' tuple which defines support march, it should
>>> arrange from lower to higher.
>>> e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'] 2. Define
>>> 'march_feature' tuple which defines support feature.
>>> e.g. 'march_feature' : ['crypto', 'sve'] 3. Select the most suitable
>>> march+feature combination based on 'march_base' and 'march_feature'
>>> tuples.
>>> 4. Use the selected march+feature combination as the default
>>> machine_args.
>>>
>>> Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")
>>>
>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>> ---
>>>  config/arm/meson.build | 27 +++++++++++++++++++++++++--
>>>  1 file changed, 25 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
>>> 3f34ec9..8551a80 100644
>>> --- a/config/arm/meson.build
>>> +++ b/config/arm/meson.build
>>> @@ -149,7 +149,9 @@ implementer_hisilicon = {
>>>      ],
>>>      'part_number_config': {
>>>          '0xd01': {
>>> -            'machine_args': ['-march=armv8.2-a+crypto', '-mtune=tsv110'],
>>> +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> If the compiler supports armv8.1-a, you need to choose armv8.1-a.
> 
>>
>> Another target has the same issue. Could you fix that all together as it is
>> generic problem in the existing infrastructure.
> I think this needs to be more generic solution. IMO, the requirement is as follows:
> 
> 1) We need to pick the most closest march_base supported by the compiler. For ex: If the SoC support v8.2 and the compiler supports v8.1, we need to pick v8.1
> 2) SoCs are allowed to support a base march version + hand picked features from the next 1 base marchs. i.e. armv8.x compliant implementation can include any features from armv8.(x + 1). Please refer to  Arm-ARM section A2 for more details. So, it is possible that the compiler supports a base march and a bunch of optional features from the next version. We need to test all the features allowed by the architecture and pick the ones that are supported in the compiler.
> 

I try to add 'march_base' : ['-march=armv8-a', '-march=armv8.5-a'] to cn10k, and then find it
can't work with ['RTE_ARM_FEATURE_ATOMICS', true] when using gcc7.2:
	[268/2250] Compiling C object lib/librte_stack.a.p/stack_rte_stack_lf.c.o
	FAILED: lib/librte_stack.a.p/stack_rte_stack_lf.c.o
	...
	{standard input}: Assembler messages:
	{standard input}:13: Error: selected processor does not support `caspl x0,x1,x2,x3,[x5]'
	[347/2250] Compiling C object lib/librte_hash.a.p/hash_rte_cuckoo_hash.c.o
	ninja: build stopped: subcommand failed.
	make: *** [cn10k] Error 1
It seem can't simplely add '-march=armv8-a' in 'march_base'.
And it require a lot of testing to get the right 'march_base' and 'march_feature' parameters.
So for v5, I just modify the kunpeng930's config which was well tested.

I think the 'march_base' and 'march_feature' could well solve the gcc's minor-version problem.
Note: the minor-version means a few version which are closes, not big gap, like gcc5.4 and gcc10.2

For that old gcc which could not support the 'march' that defined in 'machine_args' or 'march_base'
I think it better use the 'generic' profile else it will compile fail which showed above.

So how about add new tuple: fallback_generic? eg:
	'0xd02': {
            'march_base': ['-march=armv8.2-a'],
            'march_feature': ['crypto', 'sve'],
            'machine_args': [],
            'flags': [
                ['RTE_MACHINE', '"Kunpeng 930"'],
                ['RTE_ARM_FEATURE_ATOMICS', true],
                ['RTE_MAX_LCORE', 1280],
                ['RTE_MAX_NUMA_NODES', 16]
            ],
	    'fallback_generic': true
        }
PS: The premise is that the 'generic' profile is tested.


>>
>>
>>> +            'march_feature' : ['crypto'],
>>> +            'machine_args': ['-mtune=tsv110'],
>>>              'flags': [
>>>                  ['RTE_MACHINE', '"Kunpeng 920"'],
>>>                  ['RTE_ARM_FEATURE_ATOMICS', true], @@ -158,7 +160,9
>>> @@ implementer_hisilicon = {
>>>              ]
>>>          },
>>>          '0xd02': {
>>> -            'machine_args': ['-march=armv8.2-a+crypto+sve'],
>>> +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
>>> +            'march_feature' : ['crypto', 'sve'],
>>> +            'machine_args': [],
>>>              'flags': [
>>>                  ['RTE_MACHINE', '"Kunpeng 930"'],
>>>                  ['RTE_ARM_FEATURE_ATOMICS', true], @@ -449,8 +453,27
>>> @@ else
>>>      # add/overwrite flags in the proper order
>>>      dpdk_flags = flags_common + implementer_config['flags'] +
>>> part_number_config.get('flags', []) + soc_flags
>>>
>>> +    # select the most suitable march+feature combination
>>> +    machine_march = []
>>> +    if part_number_config.has_key('march_base')
>>> +        foreach arch: part_number_config['march_base']
>>> +            if cc.has_argument(arch)
>>> +                machine_march = arch # Set the higher supported arch as
>> possible
>>> +            endif
>>> +        endforeach
>>> +    endif
>>> +    if part_number_config.has_key('march_feature')
>>> +        foreach feature: part_number_config['march_feature']
>>> +            tmp_machine_march = machine_march + '+' + feature
>>> +            if cc.has_argument(tmp_machine_march)
>>> +                machine_march = tmp_machine_march # Set the more supported
>> feature as possible
>>> +            endif
>>> +        endforeach
>>> +    endif
>>> +
>>>      # apply supported machine args
>>>      machine_args = [] # Clear previous machine args
>>> +    machine_args += machine_march
>>>      foreach flag: part_number_config['machine_args']
>>>          if cc.has_argument(flag)
>>>              machine_args += flag
>>> --
>>> 2.8.1
>>>
Honnappa Nagarahalli May 18, 2021, 1:25 p.m. UTC | #4
<snip>

> >>
> >> On Thu, May 13, 2021 at 6:41 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.
> >>>
> >>> To solve this bug we use the following scheme:
> >>> 1. Define 'march_base' tuple which defines support march, it should
> >>> arrange from lower to higher.
> >>> e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'] 2. Define
> >>> 'march_feature' tuple which defines support feature.
> >>> e.g. 'march_feature' : ['crypto', 'sve'] 3. Select the most suitable
> >>> march+feature combination based on 'march_base' and 'march_feature'
> >>> tuples.
> >>> 4. Use the selected march+feature combination as the default
> >>> machine_args.
> >>>
> >>> Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")
> >>>
> >>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >>> ---
> >>>  config/arm/meson.build | 27 +++++++++++++++++++++++++--
> >>>  1 file changed, 25 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> >>> 3f34ec9..8551a80 100644
> >>> --- a/config/arm/meson.build
> >>> +++ b/config/arm/meson.build
> >>> @@ -149,7 +149,9 @@ implementer_hisilicon = {
> >>>      ],
> >>>      'part_number_config': {
> >>>          '0xd01': {
> >>> -            'machine_args': ['-march=armv8.2-a+crypto', '-mtune=tsv110'],
> >>> +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> > If the compiler supports armv8.1-a, you need to choose armv8.1-a.
> >
> >>
> >> Another target has the same issue. Could you fix that all together as
> >> it is generic problem in the existing infrastructure.
> > I think this needs to be more generic solution. IMO, the requirement is as
> follows:
> >
> > 1) We need to pick the most closest march_base supported by the
> > compiler. For ex: If the SoC support v8.2 and the compiler supports
> > v8.1, we need to pick v8.1
> > 2) SoCs are allowed to support a base march version + hand picked features
> from the next 1 base marchs. i.e. armv8.x compliant implementation can
> include any features from armv8.(x + 1). Please refer to  Arm-ARM section A2
> for more details. So, it is possible that the compiler supports a base march
> and a bunch of optional features from the next version. We need to test all
> the features allowed by the architecture and pick the ones that are supported
> in the compiler.
> >
> 
> I try to add 'march_base' : ['-march=armv8-a', '-march=armv8.5-a'] to cn10k,
> and then find it can't work with ['RTE_ARM_FEATURE_ATOMICS', true] when
> using gcc7.2:
> 	[268/2250] Compiling C object
> lib/librte_stack.a.p/stack_rte_stack_lf.c.o
> 	FAILED: lib/librte_stack.a.p/stack_rte_stack_lf.c.o
> 	...
> 	{standard input}: Assembler messages:
> 	{standard input}:13: Error: selected processor does not support `caspl
> x0,x1,x2,x3,[x5]'
> 	[347/2250] Compiling C object
> lib/librte_hash.a.p/hash_rte_cuckoo_hash.c.o
> 	ninja: build stopped: subcommand failed.
> 	make: *** [cn10k] Error 1
> It seem can't simplely add '-march=armv8-a' in 'march_base'.
> And it require a lot of testing to get the right 'march_base' and
> 'march_feature' parameters.
> So for v5, I just modify the kunpeng930's config which was well tested.
> 
> I think the 'march_base' and 'march_feature' could well solve the gcc's minor-
> version problem.
> Note: the minor-version means a few version which are closes, not big gap,
> like gcc5.4 and gcc10.2
> 
> For that old gcc which could not support the 'march' that defined in
> 'machine_args' or 'march_base'
> I think it better use the 'generic' profile else it will compile fail which showed
> above.
> 
> So how about add new tuple: fallback_generic? eg:
> 	'0xd02': {
>             'march_base': ['-march=armv8.2-a'],
>             'march_feature': ['crypto', 'sve'],
>             'machine_args': [],
>             'flags': [
>                 ['RTE_MACHINE', '"Kunpeng 930"'],
>                 ['RTE_ARM_FEATURE_ATOMICS', true],
>                 ['RTE_MAX_LCORE', 1280],
>                 ['RTE_MAX_NUMA_NODES', 16]
>             ],
> 	    'fallback_generic': true
>         }
> PS: The premise is that the 'generic' profile is tested.
Jerin, how big of a problem is this (i.e. having to compile the code with an older version of the compiler)? Is it just one old version of the compiler or there are several of them? Do they all need to be captured in the meson.build file? I am just trying to understand the need for a generic approach.

> 
> 
> >>
> >>
> >>> +            'march_feature' : ['crypto'],
> >>> +            'machine_args': ['-mtune=tsv110'],
> >>>              'flags': [
> >>>                  ['RTE_MACHINE', '"Kunpeng 920"'],
> >>>                  ['RTE_ARM_FEATURE_ATOMICS', true], @@ -158,7 +160,9
> >>> @@ implementer_hisilicon = {
> >>>              ]
> >>>          },
> >>>          '0xd02': {
> >>> -            'machine_args': ['-march=armv8.2-a+crypto+sve'],
> >>> +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> >>> +            'march_feature' : ['crypto', 'sve'],
> >>> +            'machine_args': [],
> >>>              'flags': [
> >>>                  ['RTE_MACHINE', '"Kunpeng 930"'],
> >>>                  ['RTE_ARM_FEATURE_ATOMICS', true], @@ -449,8
> >>> +453,27 @@ else
> >>>      # add/overwrite flags in the proper order
> >>>      dpdk_flags = flags_common + implementer_config['flags'] +
> >>> part_number_config.get('flags', []) + soc_flags
> >>>
> >>> +    # select the most suitable march+feature combination
> >>> +    machine_march = []
> >>> +    if part_number_config.has_key('march_base')
> >>> +        foreach arch: part_number_config['march_base']
> >>> +            if cc.has_argument(arch)
> >>> +                machine_march = arch # Set the higher supported
> >>> + arch as
> >> possible
> >>> +            endif
> >>> +        endforeach
> >>> +    endif
> >>> +    if part_number_config.has_key('march_feature')
> >>> +        foreach feature: part_number_config['march_feature']
> >>> +            tmp_machine_march = machine_march + '+' + feature
> >>> +            if cc.has_argument(tmp_machine_march)
> >>> +                machine_march = tmp_machine_march # Set the more
> >>> + supported
> >> feature as possible
> >>> +            endif
> >>> +        endforeach
> >>> +    endif
> >>> +
> >>>      # apply supported machine args
> >>>      machine_args = [] # Clear previous machine args
> >>> +    machine_args += machine_march
> >>>      foreach flag: part_number_config['machine_args']
> >>>          if cc.has_argument(flag)
> >>>              machine_args += flag
> >>> --
> >>> 2.8.1
> >>>
Jerin Jacob May 18, 2021, 1:45 p.m. UTC | #5
On Tue, May 18, 2021 at 6:55 PM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
> > >>
> > >> On Thu, May 13, 2021 at 6:41 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.
> > >>>
> > >>> To solve this bug we use the following scheme:
> > >>> 1. Define 'march_base' tuple which defines support march, it should
> > >>> arrange from lower to higher.
> > >>> e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'] 2. Define
> > >>> 'march_feature' tuple which defines support feature.
> > >>> e.g. 'march_feature' : ['crypto', 'sve'] 3. Select the most suitable
> > >>> march+feature combination based on 'march_base' and 'march_feature'
> > >>> tuples.
> > >>> 4. Use the selected march+feature combination as the default
> > >>> machine_args.
> > >>>
> > >>> Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")
> > >>>
> > >>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > >>> ---
> > >>>  config/arm/meson.build | 27 +++++++++++++++++++++++++--
> > >>>  1 file changed, 25 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > >>> 3f34ec9..8551a80 100644
> > >>> --- a/config/arm/meson.build
> > >>> +++ b/config/arm/meson.build
> > >>> @@ -149,7 +149,9 @@ implementer_hisilicon = {
> > >>>      ],
> > >>>      'part_number_config': {
> > >>>          '0xd01': {
> > >>> -            'machine_args': ['-march=armv8.2-a+crypto', '-mtune=tsv110'],
> > >>> +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> > > If the compiler supports armv8.1-a, you need to choose armv8.1-a.
> > >
> > >>
> > >> Another target has the same issue. Could you fix that all together as
> > >> it is generic problem in the existing infrastructure.
> > > I think this needs to be more generic solution. IMO, the requirement is as
> > follows:
> > >
> > > 1) We need to pick the most closest march_base supported by the
> > > compiler. For ex: If the SoC support v8.2 and the compiler supports
> > > v8.1, we need to pick v8.1
> > > 2) SoCs are allowed to support a base march version + hand picked features
> > from the next 1 base marchs. i.e. armv8.x compliant implementation can
> > include any features from armv8.(x + 1). Please refer to  Arm-ARM section A2
> > for more details. So, it is possible that the compiler supports a base march
> > and a bunch of optional features from the next version. We need to test all
> > the features allowed by the architecture and pick the ones that are supported
> > in the compiler.
> > >
> >
> > I try to add 'march_base' : ['-march=armv8-a', '-march=armv8.5-a'] to cn10k,
> > and then find it can't work with ['RTE_ARM_FEATURE_ATOMICS', true] when
> > using gcc7.2:
> >       [268/2250] Compiling C object
> > lib/librte_stack.a.p/stack_rte_stack_lf.c.o
> >       FAILED: lib/librte_stack.a.p/stack_rte_stack_lf.c.o
> >       ...
> >       {standard input}: Assembler messages:
> >       {standard input}:13: Error: selected processor does not support `caspl
> > x0,x1,x2,x3,[x5]'
> >       [347/2250] Compiling C object
> > lib/librte_hash.a.p/hash_rte_cuckoo_hash.c.o
> >       ninja: build stopped: subcommand failed.
> >       make: *** [cn10k] Error 1
> > It seem can't simplely add '-march=armv8-a' in 'march_base'.
> > And it require a lot of testing to get the right 'march_base' and
> > 'march_feature' parameters.
> > So for v5, I just modify the kunpeng930's config which was well tested.
> >
> > I think the 'march_base' and 'march_feature' could well solve the gcc's minor-
> > version problem.
> > Note: the minor-version means a few version which are closes, not big gap,
> > like gcc5.4 and gcc10.2
> >
> > For that old gcc which could not support the 'march' that defined in
> > 'machine_args' or 'march_base'
> > I think it better use the 'generic' profile else it will compile fail which showed
> > above.
> >
> > So how about add new tuple: fallback_generic? eg:
> >       '0xd02': {
> >             'march_base': ['-march=armv8.2-a'],
> >             'march_feature': ['crypto', 'sve'],
> >             'machine_args': [],
> >             'flags': [
> >                 ['RTE_MACHINE', '"Kunpeng 930"'],
> >                 ['RTE_ARM_FEATURE_ATOMICS', true],
> >                 ['RTE_MAX_LCORE', 1280],
> >                 ['RTE_MAX_NUMA_NODES', 16]
> >             ],
> >           'fallback_generic': true
> >         }
> > PS: The premise is that the 'generic' profile is tested.
> Jerin, how big of a problem is this (i.e. having to compile the code with an older version of the compiler)? Is it just one old version of the compiler or there are several of them? Do they all need to be captured in the meson.build file? I am just trying to understand the need for a generic approach.

IMO, We are supporting from gcc 4.7. So a lot of combinations are possible.


>
> >
> >
> > >>
> > >>
> > >>> +            'march_feature' : ['crypto'],
> > >>> +            'machine_args': ['-mtune=tsv110'],
> > >>>              'flags': [
> > >>>                  ['RTE_MACHINE', '"Kunpeng 920"'],
> > >>>                  ['RTE_ARM_FEATURE_ATOMICS', true], @@ -158,7 +160,9
> > >>> @@ implementer_hisilicon = {
> > >>>              ]
> > >>>          },
> > >>>          '0xd02': {
> > >>> -            'machine_args': ['-march=armv8.2-a+crypto+sve'],
> > >>> +            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> > >>> +            'march_feature' : ['crypto', 'sve'],
> > >>> +            'machine_args': [],
> > >>>              'flags': [
> > >>>                  ['RTE_MACHINE', '"Kunpeng 930"'],
> > >>>                  ['RTE_ARM_FEATURE_ATOMICS', true], @@ -449,8
> > >>> +453,27 @@ else
> > >>>      # add/overwrite flags in the proper order
> > >>>      dpdk_flags = flags_common + implementer_config['flags'] +
> > >>> part_number_config.get('flags', []) + soc_flags
> > >>>
> > >>> +    # select the most suitable march+feature combination
> > >>> +    machine_march = []
> > >>> +    if part_number_config.has_key('march_base')
> > >>> +        foreach arch: part_number_config['march_base']
> > >>> +            if cc.has_argument(arch)
> > >>> +                machine_march = arch # Set the higher supported
> > >>> + arch as
> > >> possible
> > >>> +            endif
> > >>> +        endforeach
> > >>> +    endif
> > >>> +    if part_number_config.has_key('march_feature')
> > >>> +        foreach feature: part_number_config['march_feature']
> > >>> +            tmp_machine_march = machine_march + '+' + feature
> > >>> +            if cc.has_argument(tmp_machine_march)
> > >>> +                machine_march = tmp_machine_march # Set the more
> > >>> + supported
> > >> feature as possible
> > >>> +            endif
> > >>> +        endforeach
> > >>> +    endif
> > >>> +
> > >>>      # apply supported machine args
> > >>>      machine_args = [] # Clear previous machine args
> > >>> +    machine_args += machine_march
> > >>>      foreach flag: part_number_config['machine_args']
> > >>>          if cc.has_argument(flag)
> > >>>              machine_args += flag
> > >>> --
> > >>> 2.8.1
> > >>>
>
diff mbox series

Patch

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 3f34ec9..8551a80 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -149,7 +149,9 @@  implementer_hisilicon = {
     ],
     'part_number_config': {
         '0xd01': {
-            'machine_args': ['-march=armv8.2-a+crypto', '-mtune=tsv110'],
+            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
+            'march_feature' : ['crypto'],
+            'machine_args': ['-mtune=tsv110'],
             'flags': [
                 ['RTE_MACHINE', '"Kunpeng 920"'],
                 ['RTE_ARM_FEATURE_ATOMICS', true],
@@ -158,7 +160,9 @@  implementer_hisilicon = {
             ]
         },
         '0xd02': {
-            'machine_args': ['-march=armv8.2-a+crypto+sve'],
+            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
+            'march_feature' : ['crypto', 'sve'],
+            'machine_args': [],
             'flags': [
                 ['RTE_MACHINE', '"Kunpeng 930"'],
                 ['RTE_ARM_FEATURE_ATOMICS', true],
@@ -449,8 +453,27 @@  else
     # add/overwrite flags in the proper order
     dpdk_flags = flags_common + implementer_config['flags'] + part_number_config.get('flags', []) + soc_flags
 
+    # select the most suitable march+feature combination
+    machine_march = []
+    if part_number_config.has_key('march_base')
+        foreach arch: part_number_config['march_base']
+            if cc.has_argument(arch)
+                machine_march = arch # Set the higher supported arch as possible
+            endif
+        endforeach
+    endif
+    if part_number_config.has_key('march_feature')
+        foreach feature: part_number_config['march_feature']
+            tmp_machine_march = machine_march + '+' + feature
+            if cc.has_argument(tmp_machine_march)
+                machine_march = tmp_machine_march # Set the more supported feature as possible
+            endif
+        endforeach
+    endif
+
     # apply supported machine args
     machine_args = [] # Clear previous machine args
+    machine_args += machine_march
     foreach flag: part_number_config['machine_args']
         if cc.has_argument(flag)
             machine_args += flag