Re: [dpdk-dev] [PATCH v4 1/2] config/arm: select most suitable -march for kunpeng soc

<snip>

quoted

quoted

On Thu, May 13, 2021 at 6:41 PM Chengwen Feng
[off-list ref] wrote:

quoted

Currently, the soc_kunpeng930 declares
'-march=armv8.2-a+crypto+sve', but some compiler doesn't recognize
the march because it doesn't support sve.

To solve this bug we use the following scheme:
1. Define 'march_base' tuple which defines support march, it should
arrange from lower to higher.
e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'] 2. Define
'march_feature' tuple which defines support feature.
e.g. 'march_feature' : ['crypto', 'sve'] 3. Select the most suitable
march+feature combination based on 'march_base' and 'march_feature'
tuples.
4. Use the selected march+feature combination as the default
machine_args.

Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")

Signed-off-by: Chengwen Feng <redacted>
---
 config/arm/meson.build | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build index

3f34ec9..8551a80 100644

--- a/config/arm/meson.build
+++ b/config/arm/meson.build

@@ -149,7 +149,9 @@ implementer_hisilicon = {
     ],
     'part_number_config': {
         '0xd01': {
-            'machine_args': ['-march=armv8.2-a+crypto', '-mtune=tsv110'],
+            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],

If the compiler supports armv8.1-a, you need to choose armv8.1-a.

quoted

Another target has the same issue. Could you fix that all together as
it is generic problem in the existing infrastructure.

I think this needs to be more generic solution. IMO, the requirement is as

follows:

quoted

1) We need to pick the most closest march_base supported by the
compiler. For ex: If the SoC support v8.2 and the compiler supports
v8.1, we need to pick v8.1
2) SoCs are allowed to support a base march version + hand picked features

from the next 1 base marchs. i.e. armv8.x compliant implementation can
include any features from armv8.(x + 1). Please refer to  Arm-ARM section A2
for more details. So, it is possible that the compiler supports a base march
and a bunch of optional features from the next version. We need to test all
the features allowed by the architecture and pick the ones that are supported
in the compiler.

quoted

I try to add 'march_base' : ['-march=armv8-a', '-march=armv8.5-a'] to cn10k,
and then find it can't work with ['RTE_ARM_FEATURE_ATOMICS', true] when
using gcc7.2:
	[268/2250] Compiling C object
lib/librte_stack.a.p/stack_rte_stack_lf.c.o
	FAILED: lib/librte_stack.a.p/stack_rte_stack_lf.c.o
	...
	{standard input}: Assembler messages:
	{standard input}:13: Error: selected processor does not support `caspl
x0,x1,x2,x3,[x5]'
	[347/2250] Compiling C object
lib/librte_hash.a.p/hash_rte_cuckoo_hash.c.o
	ninja: build stopped: subcommand failed.
	make: *** [cn10k] Error 1
It seem can't simplely add '-march=armv8-a' in 'march_base'.
And it require a lot of testing to get the right 'march_base' and
'march_feature' parameters.
So for v5, I just modify the kunpeng930's config which was well tested.

I think the 'march_base' and 'march_feature' could well solve the gcc's minor-
version problem.
Note: the minor-version means a few version which are closes, not big gap,
like gcc5.4 and gcc10.2

For that old gcc which could not support the 'march' that defined in
'machine_args' or 'march_base'
I think it better use the 'generic' profile else it will compile fail which showed
above.

So how about add new tuple: fallback_generic? eg:
	'0xd02': {
            'march_base': ['-march=armv8.2-a'],
            'march_feature': ['crypto', 'sve'],
            'machine_args': [],
            'flags': [
                ['RTE_MACHINE', '"Kunpeng 930"'],
                ['RTE_ARM_FEATURE_ATOMICS', true],
                ['RTE_MAX_LCORE', 1280],
                ['RTE_MAX_NUMA_NODES', 16]
            ],
	    'fallback_generic': true
        }
PS: The premise is that the 'generic' profile is tested.

Jerin, how big of a problem is this (i.e. having to compile the code with an older version of the compiler)? Is it just one old version of the compiler or there are several of them? Do they all need to be captured in the meson.build file? I am just trying to understand the need for a generic approach.

quoted

quoted

quoted

+            'march_feature' : ['crypto'],
+            'machine_args': ['-mtune=tsv110'],
             'flags': [
                 ['RTE_MACHINE', '"Kunpeng 920"'],
                 ['RTE_ARM_FEATURE_ATOMICS', true], @@ -158,7 +160,9

@@ implementer_hisilicon = {
             ]
         },
         '0xd02': {
-            'machine_args': ['-march=armv8.2-a+crypto+sve'],
+            'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
+            'march_feature' : ['crypto', 'sve'],
+            'machine_args': [],
             'flags': [
                 ['RTE_MACHINE', '"Kunpeng 930"'],
                 ['RTE_ARM_FEATURE_ATOMICS', true], @@ -449,8
+453,27 @@ else
     # add/overwrite flags in the proper order
     dpdk_flags = flags_common + implementer_config['flags'] +

part_number_config.get('flags', []) + soc_flags

+    # select the most suitable march+feature combination
+    machine_march = []
+    if part_number_config.has_key('march_base')
+        foreach arch: part_number_config['march_base']
+            if cc.has_argument(arch)
+                machine_march = arch # Set the higher supported
+ arch as

possible

quoted

+            endif
+        endforeach
+    endif
+    if part_number_config.has_key('march_feature')
+        foreach feature: part_number_config['march_feature']
+            tmp_machine_march = machine_march + '+' + feature
+            if cc.has_argument(tmp_machine_march)
+                machine_march = tmp_machine_march # Set the more
+ supported

feature as possible

quoted

+            endif
+        endforeach
+    endif
+
     # apply supported machine args
     machine_args = [] # Clear previous machine args
+    machine_args += machine_march
     foreach flag: part_number_config['machine_args']
         if cc.has_argument(flag)
             machine_args += flag
--
2.8.1