Re: [dpdk-dev] [PATCH 2/2] net/hns3: refactor SVE code compile method

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

<snip>

> >>> Currently, the SVE code is compiled only when -march supports SVE
> >>> (e.g. '- march=armv8.2a+sve'), there maybe some problem[1] with this
> >> approach.
> >>>
> >>> The solution:
> >>> a. If the minimum instruction set support SVE then compiles it.
> >>> b. Else if the compiler support SVE then compiles it.
> >>> c. Otherwise don't compile it.
> >>>
> >>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html
> >>>
> >>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
> >>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >>> ---
> >>>  drivers/net/hns3/hns3_rxtx.c |  2 +-  drivers/net/hns3/meson.build
> >>> | 13 +++++++++++++
> >>>  2 files changed, 14 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/net/hns3/hns3_rxtx.c
> >>> b/drivers/net/hns3/hns3_rxtx.c index
> >>> 1d7a769..4ef20c6 100644
> >>> --- a/drivers/net/hns3/hns3_rxtx.c
> >>> +++ b/drivers/net/hns3/hns3_rxtx.c
> >>> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void)
> >>>  static bool
> >>>  hns3_get_sve_support(void)
> >>>  {
> >>> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
> >>> +#if defined(CC_SVE_SUPPORT)
> >>>  	if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
> >>>  		return false;
> >>>  	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> >>> diff --git a/drivers/net/hns3/meson.build
> >>> b/drivers/net/hns3/meson.build index
> >>> 53c7df7..8563d70 100644
> >>> --- a/drivers/net/hns3/meson.build
> >>> +++ b/drivers/net/hns3/meson.build
> >>> @@ -35,7 +35,20 @@ deps += ['hash']
> >>>
> >>>  if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
> >>>      sources += files('hns3_rxtx_vec.c')
> >>> +
> >>> +    # compile SVE when:
> >>> +    # a. support SVE in minimum instruction set baseline
> >>> +    # b. it's not minimum instruction set, but compiler support
> >>>      if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> >>> +        cflags += ['-DCC_SVE_SUPPORT']
> >> Why is the CC_SVE_SUPPORT flag needed? The compiler has
> >> __ARM_FEATURE_SVE flag already which gets defined when '+sve" is
> >> added to '-march'.
> >>
> 
> The CC_SVE_SUPPORT is used to implement the hns3_get_sve_support API
> (below), this API located in another file which is hns3_rxtx.c, and this file was
> compiled with default machine_args.
> 
> 	static bool
> 	hns3_get_sve_support(void)
> 	{
> 	#if defined(CC_SVE_SUPPORT)
> 		if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
> 			return false;
> 		if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> 			return true;
> 	#endif
> 		return false;
> 	}
> 
> If the machine_args doesn't support SVE but compiler support, we will
> compile SVE code too, so in this case we need define CC_SVE_SUPPORT, so
> that in runtime we could try judge whether support SVE. In this case,
> __ARM_FEATURE_SVE was not defined because it is in hns3_rxtx.c which use
> default machine_args.
> 
> If the machine_args supports SVE we sure compile SVE code, in this case, to
> maintain consistency, we also define this macro.
> 
> >>>          sources += files('hns3_rxtx_vec_sve.c')
> >>> +    elif cc.has_argument('-march=armv8.2-a+sve')
> >> I think this check and the above check do the same thing. i.e. both
> >> of them check if +sve flag is passed to the compiler.
> >>
> 
> Yes it is.
> 
> >>> +        cflags += ['-DCC_SVE_SUPPORT']
> >>> +        hns3_sve_lib = static_library('hns3_sve_lib',
> >>> +                        'hns3_rxtx_vec_sve.c',
> >>> +                        dependencies: [static_rte_ethdev],
> >>> +                        include_directories: includes,
> >>> +                        c_args: [cflags, '-march=armv8.2-a+sve'])
> >>> +        objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
> > I do not understand the need of this block of code, appreciate if you could
> explain why this is required.
> >
> 
> This is typical usage for compiling some soure file with custom flags (here is '-
> march=armv8.2-a+sve') Please ref [1] with keyword 'extract_objects'
> 
> [1] https://mesonbuild.com/Reference-manual.html
Ack, understand this now, thanks for your patience

> 
> >>>      endif
> >>>  endif
> >>> --
> >>> 2.8.1
> >
> >
> > .
> >