From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: fengchengwen <fengchengwen@huawei.com>,
Ruifeng Wang <Ruifeng.Wang@arm.com>,
"thomas@monjalon.net" <thomas@monjalon.net>,
"david.marchand@redhat.com" <david.marchand@redhat.com>,
"bruce.richardson@intel.com" <bruce.richardson@intel.com>,
"Honnappa Nagarahalli" <Honnappa.Nagarahalli@arm.com>,
"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
"jerinjacobk@gmail.com" <jerinjacobk@gmail.com>,
"jerinj@marvell.com" <jerinj@marvell.com>,
Pavan Nikhilesh <pbhagavatula@marvell.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH v3] config/arm: split march cfg into arch and features
Date: Fri, 16 Jul 2021 12:22:12 +0000 [thread overview]
Message-ID: <9c672c2583954461a074f7f2798d25e0@pantheon.tech> (raw)
In-Reply-To: <26588027-5544-421f-204c-9260fa828ea0@huawei.com>
> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Friday, July 16, 2021 5:42 AM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>; Juraj Linkeš
> <juraj.linkes@pantheon.tech>; thomas@monjalon.net;
> david.marchand@redhat.com; bruce.richardson@intel.com; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; ferruh.yigit@intel.com;
> jerinjacobk@gmail.com; jerinj@marvell.com; Pavan Nikhilesh
> <pbhagavatula@marvell.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [PATCH v3] config/arm: split march cfg into arch and features
>
>
> [snip]
>
> >> +
> >> + # 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.
> >> + 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.
> >> + 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.
> >> + 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).
> >> + 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
> >
next prev parent reply other threads:[~2021-07-16 12:22 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-29 9:39 [dpdk-dev] [PATCH v1] " Juraj Linkeš
2021-06-29 9:39 ` [dpdk-dev] [PATCH v1] config/arm: split march cfg to " Juraj Linkeš
2021-07-06 8:21 ` [dpdk-dev] [PATCH v2] config/arm: split march cfg into " Juraj Linkeš
2021-07-12 13:02 ` [dpdk-dev] [PATCH v3] " Juraj Linkeš
2021-07-14 9:24 ` Ruifeng Wang
2021-07-16 3:42 ` fengchengwen
2021-07-16 12:22 ` Juraj Linkeš [this message]
2021-07-20 7:25 ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2021-07-20 11:58 ` Juraj Linkeš
2021-07-20 12:33 ` [dpdk-dev] [PATCH v3 1/2] " Juraj Linkeš
2021-07-20 12:33 ` [dpdk-dev] [PATCH v3 2/2] config/arm: make n2 'crypto' an optional feature Juraj Linkeš
2021-08-17 10:56 ` [dpdk-dev] [PATCH v4 1/2] config/arm: split march cfg into arch and features Juraj Linkeš
2021-08-17 10:56 ` [dpdk-dev] [PATCH v4 2/2] config/arm: make n2 'crypto' an optional feature Juraj Linkeš
2021-09-06 8:38 ` Ruifeng Wang
2021-08-17 10:59 ` [dpdk-dev] [PATCH v4 1/2] config/arm: split march cfg into arch and features Juraj Linkeš
2021-09-06 8:26 ` Ruifeng Wang
2021-09-16 16:17 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9c672c2583954461a074f7f2798d25e0@pantheon.tech \
--to=juraj.linkes@pantheon.tech \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=Ruifeng.Wang@arm.com \
--cc=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=ferruh.yigit@intel.com \
--cc=jerinj@marvell.com \
--cc=jerinjacobk@gmail.com \
--cc=nd@arm.com \
--cc=pbhagavatula@marvell.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).