On 2021/5/13 7:21, Honnappa Nagarahalli wrote:

<snip>

quoted

quoted

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

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 | 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.

quoted

quoted

         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.

quoted

quoted

+        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

quoted

quoted

     endif
 endif
--
2.8.1

.