diff mbox series

[2/2] net/hns3: fix SVE code compile error with gcc8.3

Message ID 1624849071-56826-3-git-send-email-fengchengwen@huawei.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series bugfix for SVE compile |

Checks

Context Check Description
ci/iol-testing success Testing PASS
ci/checkpatch success coding style OK
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Functional fail Functional Testing issues

Commit Message

fengchengwen June 28, 2021, 2:57 a.m. UTC 
  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

Ruifeng Wang June 28, 2021, 3:33 a.m. UTC | #1 
> -----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
fengchengwen June 28, 2021, 3:56 a.m. UTC | #2 
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
> 
> 
> .
>
Ruifeng Wang June 28, 2021, 5:33 a.m. UTC | #3 
> -----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
> >
> >
> > .
> >
diff mbox series

Patch

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 = []
+        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