diff mbox series

[v8,1/2] config/arm: select most suitable -march for kunpeng soc

Message ID 1621862602-51782-2-git-send-email-fengchengwen@huawei.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series bugfix for Kunpeng SVE compile |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chengwen Feng May 24, 2021, 1:23 p.m. UTC 
  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.1-a', '-march=armv8.2-a']
2. Define 'march_feature' tuple which defines support feature.
e.g. 'march_feature': ['crypto', 'sve']
Note: If user defined 'march_feature', it also needs to define a valid
'march_base' because 'march_feature' depends on 'march_base' when
checking validity.
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 <fengchengwen@huawei.com>
---
 config/arm/meson.build | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

Comments

Thomas Monjalon June 17, 2021, 7:03 a.m. UTC | #1 
24/05/2021 15:23, Chengwen Feng:
> 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.1-a', '-march=armv8.2-a']
> 2. Define 'march_feature' tuple which defines support feature.
> e.g. 'march_feature': ['crypto', 'sve']
> Note: If user defined 'march_feature', it also needs to define a valid
> 'march_base' because 'march_feature' depends on 'march_base' when
> checking validity.
> 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 <fengchengwen@huawei.com>

This patch is still not accepted.
Arm maintainers, what is missing?
Is it rejected?
Honnappa Nagarahalli June 17, 2021, 11:33 p.m. UTC | #2 
<snip>

> 
> 24/05/2021 15:23, Chengwen Feng:
> > 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.1-a', '-march=armv8.2-a'] 2. Define
> > 'march_feature' tuple which defines support feature.
> > e.g. 'march_feature': ['crypto', 'sve']
> > Note: If user defined 'march_feature', it also needs to define a valid
> > 'march_base' because 'march_feature' depends on 'march_base' when
> > checking validity.
> > 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 <fengchengwen@huawei.com>
> 
> This patch is still not accepted.
> Arm maintainers, what is missing?
> Is it rejected?
Juraj is working on a more generalized solution. Not sure how it will turn out. It would be good to wait.

> 
> 
>
Chengwen Feng June 21, 2021, 12:52 a.m. UTC | #3 
Hi, Thomas

Another patch '[dpdk-dev] [PATCH v8 2/2] net/hns3: refactor SVE code compile method'
has nothing to do with this patch (they're just in the same patchset) and has been
reviewed by ARM guys.

So please review it, thanks.


On 2021/6/18 7:33, Honnappa Nagarahalli wrote:
> <snip>
> 
>>
>> 24/05/2021 15:23, Chengwen Feng:
>>> 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.1-a', '-march=armv8.2-a'] 2. Define
>>> 'march_feature' tuple which defines support feature.
>>> e.g. 'march_feature': ['crypto', 'sve']
>>> Note: If user defined 'march_feature', it also needs to define a valid
>>> 'march_base' because 'march_feature' depends on 'march_base' when
>>> checking validity.
>>> 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 <fengchengwen@huawei.com>
>>
>> This patch is still not accepted.
>> Arm maintainers, what is missing?
>> Is it rejected?
> Juraj is working on a more generalized solution. Not sure how it will turn out. It would be good to wait.
> 
>>
>>
>>
> 
> 
> .
>
Thomas Monjalon June 23, 2021, 8:08 a.m. UTC | #4 
21/06/2021 02:52, fengchengwen:
> Hi, Thomas
> 
> Another patch '[dpdk-dev] [PATCH v8 2/2] net/hns3: refactor SVE code compile method'
> has nothing to do with this patch (they're just in the same patchset) and has been
> reviewed by ARM guys.

If they are unrelated, why are they in the same patchset?

> So please review it, thanks.
Chengwen Feng June 23, 2021, 8:24 a.m. UTC | #5 
On 2021/6/23 16:08, Thomas Monjalon wrote:
> 21/06/2021 02:52, fengchengwen:
>> Hi, Thomas
>>
>> Another patch '[dpdk-dev] [PATCH v8 2/2] net/hns3: refactor SVE code compile method'
>> has nothing to do with this patch (they're just in the same patchset) and has been
>> reviewed by ARM guys.
> 
> If they are unrelated, why are they in the same patchset?

They're all SVE-related issues, so put in the same patchset.

I will send one new patchset include following two patch because they both ref
'RTE_HAS_SVE_ACLE' which you mention previous thread.
  build: fix SVE compile error with gcc8.3
  net/hns3: refactor SVE code compile method

> 
>> So please review it, thanks.
> 
> 
> 
> 
> 
> 
> .
>
diff mbox series

Patch

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 3f34ec9..044812f 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -158,7 +158,9 @@  implementer_hisilicon = {
             ]
         },
         '0xd02': {
-            'machine_args': ['-march=armv8.2-a+crypto+sve'],
+            'march_base': ['-march=armv8.2-a'],
+            'march_feature': ['crypto', 'sve'],
+            'machine_args': [],
             'flags': [
                 ['RTE_MACHINE', '"Kunpeng 930"'],
                 ['RTE_ARM_FEATURE_ATOMICS', true],
@@ -449,8 +451,33 @@  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')
+        tmp_machine_march = ''
+        march_valid = false
+        foreach march: part_number_config['march_base']
+            if cc.has_argument(march)
+                tmp_machine_march = march # Set the higher supported march as possible
+                march_valid = true
+            endif
+        endforeach
+        # select feature only when march valid
+        if march_valid and part_number_config.has_key('march_feature')
+            foreach feature: part_number_config['march_feature']
+                tmp_feature = tmp_machine_march + '+' + feature
+                if cc.has_argument(tmp_feature)
+                    tmp_machine_march = tmp_feature # Set the more supported feature as possible
+                endif
+            endforeach
+        endif
+        if march_valid
+            machine_march = [tmp_machine_march]
+        endif
+    endif
+
     # apply supported machine args
-    machine_args = [] # Clear previous machine args
+    machine_args = machine_march # Init with machine march which set above
     foreach flag: part_number_config['machine_args']
         if cc.has_argument(flag)
             machine_args += flag