[2/2] net/hns3: fix SVE code compile error with gcc8.3
Checks
Commit Message
If the target machine has SVE feature (e.g. '-march=armv8.2-a+sve'),
and compiler are gcc8.3, it will compile error, the error is arm_sve.h
no such file or directory.
The solution:
a. If RTE_HAS_SVE_ACLE defined (it means the minimum instruction set
support SVE ACLE) then compiles it.
b. Else if the compiler support SVE ACLE then compiles it.
c. Otherwise don't compile it.
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>
Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
drivers/net/hns3/hns3_rxtx.c | 2 +-
drivers/net/hns3/meson.build | 20 +++++++++++++++++++-
2 files changed, 20 insertions(+), 2 deletions(-)
Comments
> -----Original Message-----
> From: Chengwen Feng <fengchengwen@huawei.com>
> Sent: Monday, June 28, 2021 10:58 AM
> To: thomas@monjalon.net; ferruh.yigit@intel.com; Ruifeng Wang
> <Ruifeng.Wang@arm.com>
> Cc: dev@dpdk.org; bruce.richardson@intel.com;
> vladimir.medvedkin@intel.com; viktorin@rehivetech.com;
> jerinj@marvell.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; jerinjacobk@gmail.com;
> juraj.linkes@pantheon.tech
> Subject: [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
>
> If the target machine has SVE feature (e.g. '-march=armv8.2-a+sve'), and
> compiler are gcc8.3, it will compile error, the error is arm_sve.h no such file or
> directory.
>
> The solution:
> a. If RTE_HAS_SVE_ACLE defined (it means the minimum instruction set
> support SVE ACLE) then compiles it.
> b. Else if the compiler support SVE ACLE then compiles it.
> c. Otherwise don't compile it.
>
> 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>
> Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
> drivers/net/hns3/hns3_rxtx.c | 2 +-
> drivers/net/hns3/meson.build | 20 +++++++++++++++++++-
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
> index cb9eccf..a86e105 100644
> --- a/drivers/net/hns3/hns3_rxtx.c
> +++ b/drivers/net/hns3/hns3_rxtx.c
> @@ -2811,7 +2811,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(RTE_HAS_SVE_ACLE)
> 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..a99e0db 100644
> --- a/drivers/net/hns3/meson.build
> +++ b/drivers/net/hns3/meson.build
> @@ -35,7 +35,25 @@ deps += ['hash']
>
> if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
> sources += files('hns3_rxtx_vec.c')
> - if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> +
> + # compile SVE when:
> + # a. support SVE in minimum instruction set baseline
> + # b. it's not minimum instruction set, but compiler support
> + if dpdk_conf.has('RTE_HAS_SVE_ACLE')
> sources += files('hns3_rxtx_vec_sve.c')
> + elif cc.has_argument('-march=armv8.2-a+sve') and
> cc.check_header('arm_sve.h')
> + cflags += ['-DRTE_HAS_SVE_ACLE=1']
> + sve_cflags = []
Global cflags will be changed here. I think it is not very good as build of other parts could be without SVE support.
How about " sve_cflags = ['-DRTE_HAS_SVE_ACLE=1']" and drop changes to cflags?
In this way, the additional flag will be limited to hns3_sve_lib.
> + foreach flag: cflags
> + if not (flag.startswith('-march=') or flag.startswith('-mcpu=') or
> flag.startswith('-mtune='))
> + sve_cflags += flag
> + endif
> + endforeach
> + hns3_sve_lib = static_library('hns3_sve_lib',
> + 'hns3_rxtx_vec_sve.c',
> + dependencies: [static_rte_ethdev],
> + include_directories: includes,
> + c_args: [sve_cflags, '-march=armv8.2-a+sve'])
> + objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
> endif
> endif
> --
> 2.8.1
On 2021/6/28 11:33, Ruifeng Wang wrote:
>> -----Original Message-----
>> From: Chengwen Feng <fengchengwen@huawei.com>
>> Sent: Monday, June 28, 2021 10:58 AM
>> To: thomas@monjalon.net; ferruh.yigit@intel.com; Ruifeng Wang
>> <Ruifeng.Wang@arm.com>
>> Cc: dev@dpdk.org; bruce.richardson@intel.com;
>> vladimir.medvedkin@intel.com; viktorin@rehivetech.com;
>> jerinj@marvell.com; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com>; jerinjacobk@gmail.com;
>> juraj.linkes@pantheon.tech
>> Subject: [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
>>
>> If the target machine has SVE feature (e.g. '-march=armv8.2-a+sve'), and
>> compiler are gcc8.3, it will compile error, the error is arm_sve.h no such file or
>> directory.
>>
>> The solution:
>> a. If RTE_HAS_SVE_ACLE defined (it means the minimum instruction set
>> support SVE ACLE) then compiles it.
>> b. Else if the compiler support SVE ACLE then compiles it.
>> c. Otherwise don't compile it.
>>
>> 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>
>> Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>
>> ---
>> drivers/net/hns3/hns3_rxtx.c | 2 +-
>> drivers/net/hns3/meson.build | 20 +++++++++++++++++++-
>> 2 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
>> index cb9eccf..a86e105 100644
>> --- a/drivers/net/hns3/hns3_rxtx.c
>> +++ b/drivers/net/hns3/hns3_rxtx.c
>> @@ -2811,7 +2811,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(RTE_HAS_SVE_ACLE)
>> 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..a99e0db 100644
>> --- a/drivers/net/hns3/meson.build
>> +++ b/drivers/net/hns3/meson.build
>> @@ -35,7 +35,25 @@ deps += ['hash']
>>
>> if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
>> sources += files('hns3_rxtx_vec.c')
>> - if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
>> +
>> + # compile SVE when:
>> + # a. support SVE in minimum instruction set baseline
>> + # b. it's not minimum instruction set, but compiler support
>> + if dpdk_conf.has('RTE_HAS_SVE_ACLE')
>> sources += files('hns3_rxtx_vec_sve.c')
>> + elif cc.has_argument('-march=armv8.2-a+sve') and
>> cc.check_header('arm_sve.h')
>> + cflags += ['-DRTE_HAS_SVE_ACLE=1']
>> + sve_cflags = []
> Global cflags will be changed here. I think it is not very good as build of other parts could be without SVE support.
> How about " sve_cflags = ['-DRTE_HAS_SVE_ACLE=1']" and drop changes to cflags?
> In this way, the additional flag will be limited to hns3_sve_lib.
This will not change global cflags: the cflags was independent from other drives [implemented in drivers/meson.build]
And I also check the 'build.ninja' and found the RTE_HAS_SVE_ACLE only defined with hns3 driver source file.
PS: hns3_rxtx.c also refer the marco 'RTE_HAS_SVE_ACLE'.
>
>> + foreach flag: cflags
>> + if not (flag.startswith('-march=') or flag.startswith('-mcpu=') or
>> flag.startswith('-mtune='))
>> + sve_cflags += flag
>> + endif
>> + endforeach
>> + hns3_sve_lib = static_library('hns3_sve_lib',
>> + 'hns3_rxtx_vec_sve.c',
>> + dependencies: [static_rte_ethdev],
>> + include_directories: includes,
>> + c_args: [sve_cflags, '-march=armv8.2-a+sve'])
>> + objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
>> endif
>> endif
>> --
>> 2.8.1
>
>
> .
>
> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Monday, June 28, 2021 11:56 AM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>; thomas@monjalon.net;
> ferruh.yigit@intel.com
> Cc: dev@dpdk.org; bruce.richardson@intel.com;
> vladimir.medvedkin@intel.com; viktorin@rehivetech.com;
> jerinj@marvell.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; jerinjacobk@gmail.com;
> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> Subject: Re: [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
>
> On 2021/6/28 11:33, Ruifeng Wang wrote:
> >> -----Original Message-----
> >> From: Chengwen Feng <fengchengwen@huawei.com>
> >> Sent: Monday, June 28, 2021 10:58 AM
> >> To: thomas@monjalon.net; ferruh.yigit@intel.com; Ruifeng Wang
> >> <Ruifeng.Wang@arm.com>
> >> Cc: dev@dpdk.org; bruce.richardson@intel.com;
> >> vladimir.medvedkin@intel.com; viktorin@rehivetech.com;
> >> jerinj@marvell.com; Honnappa Nagarahalli
> >> <Honnappa.Nagarahalli@arm.com>; jerinjacobk@gmail.com;
> >> juraj.linkes@pantheon.tech
> >> Subject: [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
> >>
> >> If the target machine has SVE feature (e.g. '-march=armv8.2-a+sve'),
> >> and compiler are gcc8.3, it will compile error, the error is
> >> arm_sve.h no such file or directory.
> >>
> >> The solution:
> >> a. If RTE_HAS_SVE_ACLE defined (it means the minimum instruction set
> >> support SVE ACLE) then compiles it.
> >> b. Else if the compiler support SVE ACLE then compiles it.
> >> c. Otherwise don't compile it.
> >>
> >> 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>
> >> Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>
> >> ---
> >> drivers/net/hns3/hns3_rxtx.c | 2 +- drivers/net/hns3/meson.build |
> >> 20 +++++++++++++++++++-
> >> 2 files changed, 20 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/hns3/hns3_rxtx.c
> >> b/drivers/net/hns3/hns3_rxtx.c index cb9eccf..a86e105 100644
> >> --- a/drivers/net/hns3/hns3_rxtx.c
> >> +++ b/drivers/net/hns3/hns3_rxtx.c
> >> @@ -2811,7 +2811,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(RTE_HAS_SVE_ACLE)
> >> 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..a99e0db 100644
> >> --- a/drivers/net/hns3/meson.build
> >> +++ b/drivers/net/hns3/meson.build
> >> @@ -35,7 +35,25 @@ deps += ['hash']
> >>
> >> if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
> >> sources += files('hns3_rxtx_vec.c')
> >> - if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> >> +
> >> + # compile SVE when:
> >> + # a. support SVE in minimum instruction set baseline
> >> + # b. it's not minimum instruction set, but compiler support
> >> + if dpdk_conf.has('RTE_HAS_SVE_ACLE')
> >> sources += files('hns3_rxtx_vec_sve.c')
> >> + elif cc.has_argument('-march=armv8.2-a+sve') and
> >> cc.check_header('arm_sve.h')
> >> + cflags += ['-DRTE_HAS_SVE_ACLE=1']
> >> + sve_cflags = []
> > Global cflags will be changed here. I think it is not very good as build of
> other parts could be without SVE support.
> > How about " sve_cflags = ['-DRTE_HAS_SVE_ACLE=1']" and drop changes to
> cflags?
> > In this way, the additional flag will be limited to hns3_sve_lib.
>
> This will not change global cflags: the cflags was independent from other
> drives [implemented in drivers/meson.build] And I also check the
> 'build.ninja' and found the RTE_HAS_SVE_ACLE only defined with hns3 driver
> source file.
> PS: hns3_rxtx.c also refer the marco 'RTE_HAS_SVE_ACLE'.
>
Thanks for the explanation. I see cflags is restored for each driver.
Looks good to me. My tag was already added.
> >
> >> + foreach flag: cflags
> >> + if not (flag.startswith('-march=') or
> >> + flag.startswith('-mcpu=') or
> >> flag.startswith('-mtune='))
> >> + sve_cflags += flag
> >> + endif
> >> + endforeach
> >> + hns3_sve_lib = static_library('hns3_sve_lib',
> >> + 'hns3_rxtx_vec_sve.c',
> >> + dependencies: [static_rte_ethdev],
> >> + include_directories: includes,
> >> + c_args: [sve_cflags, '-march=armv8.2-a+sve'])
> >> + objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
> >> endif
> >> endif
> >> --
> >> 2.8.1
> >
> >
> > .
> >
@@ -2811,7 +2811,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(RTE_HAS_SVE_ACLE)
if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
return false;
if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
@@ -35,7 +35,25 @@ deps += ['hash']
if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
sources += files('hns3_rxtx_vec.c')
- if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
+
+ # compile SVE when:
+ # a. support SVE in minimum instruction set baseline
+ # b. it's not minimum instruction set, but compiler support
+ if dpdk_conf.has('RTE_HAS_SVE_ACLE')
sources += files('hns3_rxtx_vec_sve.c')
+ elif cc.has_argument('-march=armv8.2-a+sve') and cc.check_header('arm_sve.h')
+ cflags += ['-DRTE_HAS_SVE_ACLE=1']
+ sve_cflags = []
+ foreach flag: cflags
+ if not (flag.startswith('-march=') or flag.startswith('-mcpu=') or flag.startswith('-mtune='))
+ sve_cflags += flag
+ endif
+ endforeach
+ hns3_sve_lib = static_library('hns3_sve_lib',
+ 'hns3_rxtx_vec_sve.c',
+ dependencies: [static_rte_ethdev],
+ include_directories: includes,
+ c_args: [sve_cflags, '-march=armv8.2-a+sve'])
+ objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
endif
endif