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

--
2.25.1
  

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
> 
> 
> .
>
  

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

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

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

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

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