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

From: Bruce Richardson <bruce.richardson@intel.com>
To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
Cc: Chengwen Feng <fengchengwen@huawei.com>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	Ruifeng Wang <Ruifeng.Wang@arm.com>,
	"viktorin@rehivetech.com" <viktorin@rehivetech.com>,
	nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH 2/2] net/hns3: refactor SVE code compile method
Date: Thu, 13 May 2021 11:04:27 +0100	[thread overview]
Message-ID: <YJz5qzx5PG0xZ7dB@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <DBAPR08MB581459DA83927BD71B8EBFD098529@DBAPR08MB5814.eurprd08.prod.outlook.com>

On Wed, May 12, 2021 at 11:12:36PM +0000, Honnappa Nagarahalli wrote:
> <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'.
> 
> >          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.
>

This is similar to what we do for AVX on x86. The first check is indeed
checking that +sve is passed to the compiler, but the second is different,
and checks whether it is possible for the flag to be passed to the
compiler. This second info then allows a separate C file to be compiled for
that extra instruction set, and then have the functions in it run-time
selected.

/Bruce 