-----Original Message-----
From: fengchengwen <redacted>
Sent: Friday, July 16, 2021 5:42 AM
To: Ruifeng Wang <redacted>; Juraj Linkeš
[off-list ref]; thomas@monjalon.net;
david.marchand@redhat.com; bruce.richardson@intel.com; Honnappa
Nagarahalli [off-list ref]; ferruh.yigit@intel.com;
jerinjacobk@gmail.com; jerinj@marvell.com; Pavan Nikhilesh
[off-list ref]
Cc: dev@dpdk.org; nd <redacted>
Subject: Re: [PATCH v3] config/arm: split march cfg into arch and features


[snip]

quoted

quoted

+
+    # probe supported marchs and their features
+    candidate_march = ''
+    if part_number_config.has_key('march')
+        supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a', 'armv8.3-a',
+                            'armv8.2-a', 'armv8.1-a', 'armv8-a']
+        check_compiler_support = false
+        foreach supported_march: supported_marchs
+            if supported_march == part_number_config['march']
+                # start checking from this version downwards
+                check_compiler_support = true
+            endif
+            if (check_compiler_support and
+                cc.has_argument('-march=' + supported_march))
+                candidate_march = supported_march
+                # highest supported march version found
+                break
+            endif

I think breaking it down into two steps is more intuitive.
step1: find the march version which target config.
       If not find then error exit.
step2: start checking from step1's version.

Ok, so you want to move the error check? Meson doesn't have good tools in terms of loops and arrays and if we move the check we'd have to either go through the list twice or have otherwise ugly code. If you know how to implement this neatly then we can change it.

quoted

quoted

+        endforeach
+        if candidate_march == ''
+            error('No suitable armv8 march version found.')
+        else

There no need use else, because meson will halt when execute error.

Ok, I'll remove it since this pattern is used elsewhere in DPDK meson.

quoted

quoted

+            if candidate_march != part_number_config['march']
+                warning('Configuration march version is ' +
+                        '@0@, but the compiler supports only @1@.'
+                        .format(part_number_config['march'], candidate_march))
+            endif
+            candidate_march = '-march=' + candidate_march
         endif
-    endforeach
+        if part_number_config.has_key('march_features')
+            feature_unsupported = false
+            foreach feature: part_number_config['march_features']
+                if cc.has_argument('+'.join([candidate_march, feature]))
+                    candidate_march = '+'.join([candidate_march, feature])
+                else
+                    feature_unsupported = true
+                endif
+            endforeach
+            if feature_unsupported
+                warning('Configuration march features are ' +
+                        '@0@, but the compiler supports only @1@.'
+                        .format(part_number_config['march_features'],
+                                candidate_march))

the march_feature is some like 'crc sve', but candidate_march is '-
march=armv8.2a+crc'.
These two displays may be a little weird because later one has -march=armv8.2
prefix.

I think it's better move warning to place which feature_unsupported was set
true.

We'll have more warning this way but it'll result in less confusing messages and nicer code - I'll make the change.

quoted

quoted

+            endif
+        endif
+        machine_args += candidate_march
+    endif
+
+    # apply supported compiler options
+    if part_number_config.has_key('compiler_options')
+        foreach flag: part_number_config['compiler_options']
+            if cc.has_argument(flag)

Is it possible that -mcpu= conflicts with -march ?

Maybe, but I think this sort of check is out of scope of this change. We can do this in a separate patch if there's a need (and if it's actually possible to do the check).

quoted

quoted

+                machine_args += flag
+            else
+                warning('Configuration compiler option ' +
+                        '@0@ isn\'t supported.'.format(flag))
+            endif
+        endforeach
+    endif

     # apply flags
     foreach flag: dpdk_flags
--
2.20.1