[PATCH 1/2] config/arm: add SVE control flag

[PATCH 1/2] config/arm: add SVE control flag

[Rahul Bhansali]

This add the control flag for SVE to enable or disable
RTE_HAS_SVE_ACLE macro in the build.

Signed-off-by: Rahul Bhansali <rbhansali@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 8aead74086..dafb342cc6 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -603,7 +603,8 @@ if (cc.get_define('__ARM_NEON', args: machine_args) != '' or
     compile_time_cpuflags += ['RTE_CPUFLAG_NEON']
 endif
 
-if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
+if (cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' and
+    soc_config.get('sve', true))
     compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
     if (cc.check_header('arm_sve.h'))
         dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
-- 
2.25.1

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

[Rahul Bhansali]

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@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
 }
 
 soc_dpaa = {
-- 
2.25.1

Re: [PATCH 1/2] config/arm: add SVE control flag

[Bruce Richardson]

On Thu, May 05, 2022 at 07:57:43PM +0530, Rahul Bhansali wrote:
> This add the control flag for SVE to enable or disable
> RTE_HAS_SVE_ACLE macro in the build.
> 
> Signed-off-by: Rahul Bhansali <rbhansali@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 8aead74086..dafb342cc6 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -603,7 +603,8 @@ if (cc.get_define('__ARM_NEON', args: machine_args) != '' or
>      compile_time_cpuflags += ['RTE_CPUFLAG_NEON']
>  endif
>  
> -if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> +if (cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' and
> +    soc_config.get('sve', true))

Please double-indent this so that it does not line up with the following
lines of the block.

>      compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
>      if (cc.check_header('arm_sve.h'))
>          dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
> -- 
> 2.25.1
> 

Re: [PATCH 1/2] config/arm: add SVE control flag

[fengchengwen]

On 2022/5/5 22:27, Rahul Bhansali wrote:
> This add the control flag for SVE to enable or disable
> RTE_HAS_SVE_ACLE macro in the build.
> 
> Signed-off-by: Rahul Bhansali <rbhansali@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 8aead74086..dafb342cc6 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -603,7 +603,8 @@ if (cc.get_define('__ARM_NEON', args: machine_args) != '' or
>      compile_time_cpuflags += ['RTE_CPUFLAG_NEON']
>  endif
>  
> -if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> +if (cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' and
> +    soc_config.get('sve', true))

The march_features already contain the flag.
If the cpu support 'sve' then it could declare it in march_features, please refer Kunpeng 930:

        '0xd02': {
            'march': 'armv8.2-a',
            'march_features': ['crypto', 'sve'],
            'flags': [
                ['RTE_MACHINE', '"Kunpeng 930"'],
                ['RTE_ARM_FEATURE_ATOMICS', true],
                ['RTE_MAX_LCORE', 1280],
                ['RTE_MAX_NUMA_NODES', 16]
            ]
        }

>      compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
>      if (cc.check_header('arm_sve.h'))
>          dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
> 

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

[fengchengwen]

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@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]
        ]
    }

>  }
>  
>  soc_dpaa = {
> 

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

[Rahul Bhansali]

> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Friday, May 6, 2022 8:00 AM
> To: Rahul Bhansali <rbhansali@marvell.com>; dev@dpdk.org; Ruifeng Wang
> <ruifeng.wang@arm.com>; Jan Viktorin <viktorin@rehivetech.com>; Bruce
> Richardson <bruce.richardson@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@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@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.

> >  }
> >
> >  soc_dpaa = {
> >

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

[fengchengwen]

On 2022/5/6 12:54, Rahul Bhansali wrote:
> 
> 
>> -----Original Message-----
>> From: fengchengwen <fengchengwen@huawei.com>
>> Sent: Friday, May 6, 2022 8:00 AM
>> To: Rahul Bhansali <rbhansali@marvell.com>; dev@dpdk.org; Ruifeng Wang
>> <ruifeng.wang@arm.com>; Jan Viktorin <viktorin@rehivetech.com>; Bruce
>> Richardson <bruce.richardson@intel.com>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@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@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.
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.

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

> 
>>>  }
>>>
>>>  soc_dpaa = {
>>>
> 

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

[Ruifeng Wang]

> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Friday, May 6, 2022 2:36 PM
> To: Rahul Bhansali <rbhansali@marvell.com>; dev@dpdk.org; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Jan Viktorin <viktorin@rehivetech.com>; Bruce
> Richardson <bruce.richardson@intel.com>
> Cc: jerinj@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@huawei.com>
> >> Sent: Friday, May 6, 2022 8:00 AM
> >> To: Rahul Bhansali <rbhansali@marvell.com>; dev@dpdk.org; Ruifeng
> >> Wang <ruifeng.wang@arm.com>; Jan Viktorin
> <viktorin@rehivetech.com>;
> >> Bruce Richardson <bruce.richardson@intel.com>
> >> Cc: Jerin Jacob Kollanukkaran <jerinj@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@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.

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

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

[Rahul Bhansali]

> -----Original Message-----
> From: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Sent: Friday, May 6, 2022 12:53 PM
> To: fengchengwen <fengchengwen@huawei.com>; Rahul Bhansali
> <rbhansali@marvell.com>; dev@dpdk.org; Jan Viktorin
> <viktorin@rehivetech.com>; Bruce Richardson <bruce.richardson@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; nd <nd@arm.com>
> Subject: RE: [EXT] Re: [PATCH 2/2] config/arm: disable SVE for cn10k
> 
> > -----Original Message-----
> > From: fengchengwen <fengchengwen@huawei.com>
> > Sent: Friday, May 6, 2022 2:36 PM
> > To: Rahul Bhansali <rbhansali@marvell.com>; dev@dpdk.org; Ruifeng Wang
> > <Ruifeng.Wang@arm.com>; Jan Viktorin <viktorin@rehivetech.com>; Bruce
> > Richardson <bruce.richardson@intel.com>
> > Cc: jerinj@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@huawei.com>
> > >> Sent: Friday, May 6, 2022 8:00 AM
> > >> To: Rahul Bhansali <rbhansali@marvell.com>; dev@dpdk.org; Ruifeng
> > >> Wang <ruifeng.wang@arm.com>; Jan Viktorin
> > <viktorin@rehivetech.com>;
> > >> Bruce Richardson <bruce.richardson@intel.com>
> > >> Cc: Jerin Jacob Kollanukkaran <jerinj@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@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.

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

RE: [EXT] Re: [PATCH 1/2] config/arm: add SVE control flag

[Rahul Bhansali]

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, May 5, 2022 8:09 PM
> To: Rahul Bhansali <rbhansali@marvell.com>
> Cc: dev@dpdk.org; Ruifeng Wang <ruifeng.wang@arm.com>; Jan Viktorin
> <viktorin@rehivetech.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Subject: [EXT] Re: [PATCH 1/2] config/arm: add SVE control flag
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Thu, May 05, 2022 at 07:57:43PM +0530, Rahul Bhansali wrote:
> > This add the control flag for SVE to enable or disable
> > RTE_HAS_SVE_ACLE macro in the build.
> >
> > Signed-off-by: Rahul Bhansali <rbhansali@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
> > 8aead74086..dafb342cc6 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -603,7 +603,8 @@ if (cc.get_define('__ARM_NEON', args: machine_args)
> != '' or
> >      compile_time_cpuflags += ['RTE_CPUFLAG_NEON']  endif
> >
> > -if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> > +if (cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' and
> > +    soc_config.get('sve', true))
> 
> Please double-indent this so that it does not line up with the following lines of
> the block.
> 

Ack, will send v2 with this double-indent change.

> >      compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
> >      if (cc.check_header('arm_sve.h'))
> >          dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
> > --
> > 2.25.1
> >

Re: [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@arm.com>
>> Sent: Friday, May 6, 2022 12:53 PM
>> To: fengchengwen <fengchengwen@huawei.com>; Rahul Bhansali
>> <rbhansali@marvell.com>; dev@dpdk.org; Jan Viktorin
>> <viktorin@rehivetech.com>; Bruce Richardson <bruce.richardson@intel.com>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; nd <nd@arm.com>
>> Subject: RE: [EXT] Re: [PATCH 2/2] config/arm: disable SVE for cn10k
>>
>>> -----Original Message-----
>>> From: fengchengwen <fengchengwen@huawei.com>
>>> Sent: Friday, May 6, 2022 2:36 PM
>>> To: Rahul Bhansali <rbhansali@marvell.com>; dev@dpdk.org; Ruifeng Wang
>>> <Ruifeng.Wang@arm.com>; Jan Viktorin <viktorin@rehivetech.com>; Bruce
>>> Richardson <bruce.richardson@intel.com>
>>> Cc: jerinj@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@huawei.com>
>>>>> Sent: Friday, May 6, 2022 8:00 AM
>>>>> To: Rahul Bhansali <rbhansali@marvell.com>; dev@dpdk.org; Ruifeng
>>>>> Wang <ruifeng.wang@arm.com>; Jan Viktorin
>>> <viktorin@rehivetech.com>;
>>>>> Bruce Richardson <bruce.richardson@intel.com>
>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@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@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 = {
>>>>>>
>>>>
> 

[PATCH v2 1/2] config/arm: add SVE ACLE control flag

[Rahul Bhansali]

This add the control flag for SVE ACLE to enable or disable
RTE_HAS_SVE_ACLE macro in the build.

Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
---
Changes in v2:
- Renamed the flag to sve_acle from sve
- Added double-indent.

 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 8aead74086..2e12b541ef 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -603,7 +603,8 @@ if (cc.get_define('__ARM_NEON', args: machine_args) != '' or
     compile_time_cpuflags += ['RTE_CPUFLAG_NEON']
 endif

-if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
+if (cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' and
+        soc_config.get('sve_acle', true))
     compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
     if (cc.check_header('arm_sve.h'))
         dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
--
2.25.1

[PATCH v2 2/2] config/arm: disable SVE ACLE for cn10k

[Rahul Bhansali]

This disable the sve_acle 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@marvell.com>
---
Changes in v2:
- Renamed the flag to sve_acle from sve

 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 2e12b541ef..2177e2e250 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_acle': false
 }

 soc_dpaa = {
--
2.25.1

Re: [PATCH v2 1/2] config/arm: add SVE ACLE control flag

[fengchengwen]

On 2022/5/7 17:39, Rahul Bhansali wrote:
> This add the control flag for SVE ACLE to enable or disable
> RTE_HAS_SVE_ACLE macro in the build.
> 
> Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
> ---
> Changes in v2:
> - Renamed the flag to sve_acle from sve
> - Added double-indent.
> 
>  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 8aead74086..2e12b541ef 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -603,7 +603,8 @@ if (cc.get_define('__ARM_NEON', args: machine_args) != '' or
>      compile_time_cpuflags += ['RTE_CPUFLAG_NEON']
>  endif
> 
> -if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> +if (cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' and
> +        soc_config.get('sve_acle', true))
>      compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
>      if (cc.check_header('arm_sve.h'))

the above sve_acle also impact RTE_CPUFLAG_SVE which don't mentioned in commit log.
suggest add soc_config.get('sve_acle', true) in above if statement.

>          dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
> --
> 2.25.1
> 
> 
> .
> 

[PATCH v3 1/2] config/arm: add SVE ACLE control flag

[Rahul Bhansali]

This add the control flag for SVE ACLE to enable or disable
RTE_HAS_SVE_ACLE macro in the build.

Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
---
Changes in v3:
- Moved sve_acle condition to be consider for
RTE_HAS_SVE_ACLE flag only.

Changes in v2:
- Renamed the flag to sve_acle from sve
- Added double-indent.

 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 8aead74086..6f8961eac8 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -605,7 +605,7 @@ endif

 if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
     compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
-    if (cc.check_header('arm_sve.h'))
+    if (cc.check_header('arm_sve.h') and soc_config.get('sve_acle', true))
         dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
     endif
 endif
--
2.25.1

[PATCH v3 2/2] config/arm: disable SVE ACLE for cn10k

[Rahul Bhansali]

This disable the sve_acle 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@marvell.com>
---
Changes in v3: No change

Changes in v2:
- Renamed the flag to sve_acle from sve

 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 6f8961eac8..a94129168f 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_acle': false
 }

 soc_dpaa = {
--
2.25.1

[PATCH v4 1/2] config/arm: add SVE ACLE control flag

[Rahul Bhansali]

This add the control flag for SVE ACLE to enable or disable
RTE_HAS_SVE_ACLE macro in the build.

Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
---
Changes in v4:
- Resend patches. With v3, patches were not sent properly
in single series.

Changes in v3:
- Moved sve_acle condition to be consider for
RTE_HAS_SVE_ACLE flag only.

Changes in v2:
- Renamed the flag to sve_acle from sve
- Added double-indent.

 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 8aead74086..6f8961eac8 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -605,7 +605,7 @@ endif

 if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
     compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
-    if (cc.check_header('arm_sve.h'))
+    if (cc.check_header('arm_sve.h') and soc_config.get('sve_acle', true))
         dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
     endif
 endif
--
2.25.1

[PATCH v4 2/2] config/arm: disable SVE ACLE for cn10k

[Rahul Bhansali]

This disable the sve_acle 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@marvell.com>
---
Changes in v4:
- Resend patches. With v3, patches were not sent properly
in single series.

Changes in v3: No change

Changes in v2:
- Renamed the flag to sve_acle from sve

 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 6f8961eac8..a94129168f 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_acle': false
 }

 soc_dpaa = {
--
2.25.1

Re: [PATCH v4 1/2] config/arm: add SVE ACLE control flag

[fengchengwen]

Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>

On 2022/5/9 18:19, Rahul Bhansali wrote:
> This add the control flag for SVE ACLE to enable or disable
> RTE_HAS_SVE_ACLE macro in the build.
> 
> Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
> ---
> Changes in v4:
> - Resend patches. With v3, patches were not sent properly
> in single series.
> 
> Changes in v3:
> - Moved sve_acle condition to be consider for
> RTE_HAS_SVE_ACLE flag only.
> 
> Changes in v2:
> - Renamed the flag to sve_acle from sve
> - Added double-indent.
> 
>  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 8aead74086..6f8961eac8 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -605,7 +605,7 @@ endif
> 
>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
>      compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
> -    if (cc.check_header('arm_sve.h'))
> +    if (cc.check_header('arm_sve.h') and soc_config.get('sve_acle', true))
>          dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
>      endif
>  endif
> --
> 2.25.1
> 
> 
> .
> 

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

[fengchengwen]

Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>

On 2022/5/9 18:19, Rahul Bhansali wrote:
> This disable the sve_acle 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@marvell.com>
> ---
> Changes in v4:
> - Resend patches. With v3, patches were not sent properly
> in single series.
> 
> Changes in v3: No change
> 
> Changes in v2:
> - Renamed the flag to sve_acle from sve
> 
>  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 6f8961eac8..a94129168f 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_acle': false
>  }
> 
>  soc_dpaa = {
> --
> 2.25.1
> 
> 
> .
> 

RE: [PATCH v4 1/2] config/arm: add SVE ACLE control flag

[Ruifeng Wang]

> -----Original Message-----
> From: Rahul Bhansali <rbhansali@marvell.com>
> Sent: Monday, May 9, 2022 6:20 PM
> To: dev@dpdk.org; Ruifeng Wang <Ruifeng.Wang@arm.com>; Jan Viktorin
> <viktorin@rehivetech.com>; Bruce Richardson <bruce.richardson@intel.com>
> Cc: jerinj@marvell.com; Rahul Bhansali <rbhansali@marvell.com>
> Subject: [PATCH v4 1/2] config/arm: add SVE ACLE control flag
> 
> This add the control flag for SVE ACLE to enable or disable
> RTE_HAS_SVE_ACLE macro in the build.
> 
> Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
> ---
Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>

RE: [PATCH v4 2/2] config/arm: disable SVE ACLE for cn10k

[Ruifeng Wang]

> -----Original Message-----
> From: Rahul Bhansali <rbhansali@marvell.com>
> Sent: Monday, May 9, 2022 6:20 PM
> To: dev@dpdk.org; Jan Viktorin <viktorin@rehivetech.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Bruce Richardson <bruce.richardson@intel.com>
> Cc: jerinj@marvell.com; Rahul Bhansali <rbhansali@marvell.com>
> Subject: [PATCH v4 2/2] config/arm: disable SVE ACLE for cn10k
> 
> This disable the sve_acle 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@marvell.com>
> ---
Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>

RE: [PATCH v4 1/2] config/arm: add SVE ACLE control flag

[Honnappa Nagarahalli]

<snip>

> >
> > This add the control flag for SVE ACLE to enable or disable
       ^^^ typo? 
> > RTE_HAS_SVE_ACLE macro in the build.
> >
> > Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
> > ---
> Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>

RE: [PATCH v4 2/2] config/arm: disable SVE ACLE for cn10k

[Honnappa Nagarahalli]

<snip>
> > -----Original Message-----
> > From: Rahul Bhansali <rbhansali@marvell.com>
> > Sent: Monday, May 9, 2022 6:20 PM
> > To: dev@dpdk.org; Jan Viktorin <viktorin@rehivetech.com>; Ruifeng Wang
> > <Ruifeng.Wang@arm.com>; Bruce Richardson
> <bruce.richardson@intel.com>
> > Cc: jerinj@marvell.com; Rahul Bhansali <rbhansali@marvell.com>
> > Subject: [PATCH v4 2/2] config/arm: disable SVE ACLE for cn10k
> >
> > This disable the sve_acle flag for cn10k.
> >
> > Performance impact:-
> > With l3fwd example, lpm lookup performance increased by ~21% if Neon
> > is used instead of SVE.
Suggest changing this commit log as follows:
"With l3fwd example, lpm lookup performance increased by ~21% if Neon is used instead of SVE. Hence, disable sve_acle flag for cn10k".

> >
> > Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
> > ---
> Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>

RE: [PATCH v4 1/2] config/arm: add SVE ACLE control flag

[Juraj Linkeš]

> -----Original Message-----
> From: Rahul Bhansali <rbhansali@marvell.com>
> Sent: Monday, May 9, 2022 12:20 PM
> To: dev@dpdk.org; Ruifeng Wang <ruifeng.wang@arm.com>; Jan Viktorin
> <viktorin@rehivetech.com>; Bruce Richardson <bruce.richardson@intel.com>
> Cc: jerinj@marvell.com; Rahul Bhansali <rbhansali@marvell.com>
> Subject: [PATCH v4 1/2] config/arm: add SVE ACLE control flag
> 
> This add the control flag for SVE ACLE to enable or disable RTE_HAS_SVE_ACLE
> macro in the build.
> 
> Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
> ---
> Changes in v4:
> - Resend patches. With v3, patches were not sent properly in single series.
> 
> Changes in v3:
> - Moved sve_acle condition to be consider for RTE_HAS_SVE_ACLE flag only.
> 
> Changes in v2:
> - Renamed the flag to sve_acle from sve
> - Added double-indent.
> 
>  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
> 8aead74086..6f8961eac8 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -605,7 +605,7 @@ endif
> 
>  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
>      compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
> -    if (cc.check_header('arm_sve.h'))
> +    if (cc.check_header('arm_sve.h') and soc_config.get('sve_acle',
> + true))

This configuration will be applied only for non-native builds - when we specify either -Dplatform or do a cross-build (with the target being cn10k). Is that what we want? I'm not sure how we'd do that for native builds that won't affect non-cn10k builds, as we can do this either at the implementer or part number level (both of which cover other SoCs).

>          dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
>      endif
>  endif
> --
> 2.25.1
> 

RE: [PATCH v4 1/2] config/arm: add SVE ACLE control flag

[Rahul Bhansali]

> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Tuesday, May 17, 2022 1:26 PM
> To: Rahul Bhansali <rbhansali@marvell.com>; dev@dpdk.org; Ruifeng Wang
> <ruifeng.wang@arm.com>; Jan Viktorin <viktorin@rehivetech.com>; Bruce
> Richardson <bruce.richardson@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Subject: [EXT] RE: [PATCH v4 1/2] config/arm: add SVE ACLE control flag
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> > -----Original Message-----
> > From: Rahul Bhansali <rbhansali@marvell.com>
> > Sent: Monday, May 9, 2022 12:20 PM
> > To: dev@dpdk.org; Ruifeng Wang <ruifeng.wang@arm.com>; Jan Viktorin
> > <viktorin@rehivetech.com>; Bruce Richardson
> > <bruce.richardson@intel.com>
> > Cc: jerinj@marvell.com; Rahul Bhansali <rbhansali@marvell.com>
> > Subject: [PATCH v4 1/2] config/arm: add SVE ACLE control flag
> >
> > This add the control flag for SVE ACLE to enable or disable
> > RTE_HAS_SVE_ACLE macro in the build.
> >
> > Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
> > ---
> > Changes in v4:
> > - Resend patches. With v3, patches were not sent properly in single series.
> >
> > Changes in v3:
> > - Moved sve_acle condition to be consider for RTE_HAS_SVE_ACLE flag only.
> >
> > Changes in v2:
> > - Renamed the flag to sve_acle from sve
> > - Added double-indent.
> >
> >  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
> > 8aead74086..6f8961eac8 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -605,7 +605,7 @@ endif
> >
> >  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> >      compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
> > -    if (cc.check_header('arm_sve.h'))
> > +    if (cc.check_header('arm_sve.h') and soc_config.get('sve_acle',
> > + true))
> 
> This configuration will be applied only for non-native builds - when we specify
> either -Dplatform or do a cross-build (with the target being cn10k). Is that what
> we want? I'm not sure how we'd do that for native builds that won't affect non-
> cn10k builds, as we can do this either at the implementer or part number level
> (both of which cover other SoCs).
> 

For native build, we will need to specify -Dplatform for cn10k so that sve_acle can be disabled for this only. Currently performance impact of SVE_ACLE vs Neon is checked on cn10k only, not sure about other platforms, hence not done default SVE ACLE disabled for all platforms.
For cn10k, implementor and part number is same as with N2, so not done required changes at that level. 

In future, if performance impact is same for other platforms too then we can have this solution based on implementor ID or part number.

> >          dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
> >      endif
> >  endif
> > --
> > 2.25.1
> >
> 

RE: [PATCH v4 1/2] config/arm: add SVE ACLE control flag

[Juraj Linkeš]

> > > -----Original Message-----
> > > From: Rahul Bhansali <rbhansali@marvell.com>
> > > Sent: Monday, May 9, 2022 12:20 PM
> > > To: dev@dpdk.org; Ruifeng Wang <ruifeng.wang@arm.com>; Jan Viktorin
> > > <viktorin@rehivetech.com>; Bruce Richardson
> > > <bruce.richardson@intel.com>
> > > Cc: jerinj@marvell.com; Rahul Bhansali <rbhansali@marvell.com>
> > > Subject: [PATCH v4 1/2] config/arm: add SVE ACLE control flag
> > >
> > > This add the control flag for SVE ACLE to enable or disable
> > > RTE_HAS_SVE_ACLE macro in the build.
> > >
> > > Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
> > > ---
> > > Changes in v4:
> > > - Resend patches. With v3, patches were not sent properly in single series.
> > >
> > > Changes in v3:
> > > - Moved sve_acle condition to be consider for RTE_HAS_SVE_ACLE flag only.
> > >
> > > Changes in v2:
> > > - Renamed the flag to sve_acle from sve
> > > - Added double-indent.
> > >
> > >  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
> > > 8aead74086..6f8961eac8 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -605,7 +605,7 @@ endif
> > >
> > >  if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> > >      compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
> > > -    if (cc.check_header('arm_sve.h'))
> > > +    if (cc.check_header('arm_sve.h') and soc_config.get('sve_acle',
> > > + true))
> >
> > This configuration will be applied only for non-native builds - when
> > we specify either -Dplatform or do a cross-build (with the target
> > being cn10k). Is that what we want? I'm not sure how we'd do that for
> > native builds that won't affect non- cn10k builds, as we can do this
> > either at the implementer or part number level (both of which cover other
> SoCs).
> >
> 
> For native build, we will need to specify -Dplatform for cn10k so that sve_acle
> can be disabled for this only. Currently performance impact of SVE_ACLE vs
> Neon is checked on cn10k only, not sure about other platforms, hence not done
> default SVE ACLE disabled for all platforms.
> For cn10k, implementor and part number is same as with N2, so not done
> required changes at that level.
> 
Ok, it looks like you've thought about the native build case and have it covered.

> In future, if performance impact is same for other platforms too then we can
> have this solution based on implementor ID or part number.
> 
Makes sense, as I suspected we can't do this change more broadly and have to use -Dplatform even for native builds.

> > >          dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
> > >      endif
> > >  endif
> > > --
> > > 2.25.1
> > >
> >
> 

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

[PATCH v5 1/2] config/arm: add SVE ACLE control flag

[Rahul Bhansali]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 1276 bytes --]

An additional check of control flag sve_acle for
RTE_HAS_SVE_ACLE macro to be part of the build.
If any SoC config doesn't have sve_acle flag parameter
then default it will be considered as true.

Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
Changes in v5: Updated commit message

Changes in v4:
- Resend patches. With v3, patches were not sent properly
in single series.

Changes in v3:
- Moved sve_acle condition to be consider for
RTE_HAS_SVE_ACLE flag only.

Changes in v2:
- Renamed the flag to sve_acle from sve
- Added double-indent.

 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 8aead74086..6f8961eac8 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -605,7 +605,7 @@ endif

 if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
     compile_time_cpuflags += ['RTE_CPUFLAG_SVE']
-    if (cc.check_header('arm_sve.h'))
+    if (cc.check_header('arm_sve.h') and soc_config.get('sve_acle', true))
         dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
     endif
 endif
--
2.25.1

[PATCH v5 2/2] config/arm: disable SVE ACLE for cn10k

[Rahul Bhansali]

This disable the sve_acle flag for cn10k.

For native build, -Dplatform=cn10k will require to
get sve_acle flag parameter in the build.

Performance impact:-
With l3fwd example, lpm lookup performance increased
by ~21% if Neon is used instead of SVE. Hence, disabled
sve_acle flag for cn10k.

Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
Changes in v5: Updated commit message.

Changes in v4:
- Resend patches. With v3, patches were not sent properly
in single series.

Changes in v3: No change

Changes in v2:
- Renamed the flag to sve_acle from sve

 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 6f8961eac8..a94129168f 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_acle': false
 }

 soc_dpaa = {
--
2.25.1

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

[Thomas Monjalon]

19/05/2022 15:28, Rahul Bhansali:
> This disable the sve_acle flag for cn10k.
> 
> For native build, -Dplatform=cn10k will require to
> get sve_acle flag parameter in the build.
> 
> Performance impact:-
> With l3fwd example, lpm lookup performance increased
> by ~21% if Neon is used instead of SVE. Hence, disabled
> sve_acle flag for cn10k.
> 
> Signed-off-by: Rahul Bhansali <rbhansali@marvell.com>
> Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>

Series applied, thanks.