-----Original Message-----
From: fengchengwen <redacted>
Sent: Friday, May 21, 2021 2:53 PM
To: Ruifeng Wang <redacted>; thomas@monjalon.net;
ferruh.yigit@intel.com
Cc: dev@dpdk.org; jerinj@marvell.com; viktorin@rehivetech.com;
bruce.richardson@intel.com; Honnappa Nagarahalli
[off-list ref]; jerinjacobk@gmail.com;
juraj.linkes@pantheon.tech; nd [off-list ref]
Subject: Re: [PATCH v6 2/2] net/hns3: refactor SVE code compile method



On 2021/5/21 13:21, Ruifeng Wang wrote:

quoted

quoted

-----Original Message-----
From: fengchengwen <redacted>
Sent: Thursday, May 20, 2021 6:55 PM
To: Ruifeng Wang <redacted>; thomas@monjalon.net;
ferruh.yigit@intel.com
Cc: dev@dpdk.org; jerinj@marvell.com; viktorin@rehivetech.com;
bruce.richardson@intel.com; Honnappa Nagarahalli
[off-list ref]; jerinjacobk@gmail.com;
juraj.linkes@pantheon.tech; nd [off-list ref]
Subject: Re: [PATCH v6 2/2] net/hns3: refactor SVE code compile
method



On 2021/5/20 16:24, Ruifeng Wang wrote:

quoted

quoted

-----Original Message-----
From: Chengwen Feng <redacted>
Sent: Wednesday, May 19, 2021 9:26 PM
To: thomas@monjalon.net; ferruh.yigit@intel.com
Cc: dev@dpdk.org; jerinj@marvell.com; Ruifeng Wang
[off-list ref]; viktorin@rehivetech.com;
bruce.richardson@intel.com; Honnappa Nagarahalli
[off-list ref]; jerinjacobk@gmail.com;
juraj.linkes@pantheon.tech; nd [off-list ref]
Subject: [PATCH v6 2/2] net/hns3: refactor SVE code compile method

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.

quoted

quoted

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 <redacted>
---
 drivers/net/hns3/hns3_rxtx.c |  2 +-  drivers/net/hns3/meson.build
|
21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

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..5f9af9b 100644

--- a/drivers/net/hns3/meson.build
+++ b/drivers/net/hns3/meson.build

@@ -35,7 +35,26 @@ 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 cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
+ and

cc.check_header('arm_sve.h')
+        cflags += ['-DCC_SVE_SUPPORT']

With SVE build fix patch [1], CC_SVE_ACLE_SUPPORT will be defined.
Here we can use CC_SVE_ACLE_SUPPORT and not to add a new one.

The CC_SVE_ACLE_SUPPORT was defined under default machine_args

which

quoted

quoted

support SVE, it can't deals with the situation: the default
machine_args don't support SVE but compiler support SVE.
So the CC_SVE_SUPPORT marco is necessary.

Agree that macro for SVE is also needed here. And we can also use '-

DCC_SVE_ACLE_SUPPORT' here right?

quoted

I think there is no difference between CC_SVE_ACLE_SUPPORT and

CC_SVE_SUPPORT when they are used in source code.

quoted

IMO the same macro name can be used, and it removes redundancy and

confusion.

quoted

You are right, no difference between CC_SVE_ACLE_SUPPORT and
CC_SVE_SUPPORT But the hns3 sve already support 20.11, and
CC_SVE_ACLE_SUPPORT was newly defined, there maybe some problems
when backporting.

20.11 release has no machine enabled SVE extension. 

Or we could redefine CC_SVE_ACLE_SUPPORT under default machine_args:
    if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' and
cc.check_header('arm_sve.h')
        cflags += ['-DCC_SVE_ACLE_SUPPORT']

'if dpdk_conf.get(CC_SVE_ACLE_SUPPORT)' should be fine?
Stable branch has no SVE enabled in machine_args.

        sources += files('hns3_rxtx_vec_sve.c')
    elif cc.has_argument('-march=armv8.2-a+sve') and
cc.check_header('arm_sve.h')
        sve_cflags = ['-DCC_SVE_ACLE_SUPPORT']

This is fine. Macro name is consistent.

        foreach flag: cflags
            # filterout -march -mcpu -mtune
            if not (flag.startswith('-march=') or flag.startswith('-mcpu=') or
flag.startswith('-mtune='))
                sve_cflags += flag
            endif
        endforeach
but this way may introduce coupling, I think.

quoted

quoted

quoted

[1]
http://patches.dpdk.org/project/dpdk/patch/1621495007-28387-1-git-se
nd
-email-fengchengwen@huawei.com/

quoted

         sources += files('hns3_rxtx_vec_sve.c')
+    elif cc.has_argument('-march=armv8.2-a+sve') and
cc.check_header('arm_sve.h')
+        sve_cflags = ['-DCC_SVE_SUPPORT']
+        foreach flag: cflags
+            # filterout -march -mcpu -mtune
+            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

.