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

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

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