[1/2] config/arm: avoid mcpu and march conflicts
Checks
Commit Message
From: Pavan Nikhilesh <pbhagavatula@marvell.com>
The compiler options march and mtune are a subset
of mcpu and will lead to conflicts if improper march
is chosen for a given mcpu.
To avoid conflicts, force part number march when
mcpu is available and is supported by the compiler.
Example:
march = armv9-a
mcpu = neoverse-n2
mcpu supported, march supported
machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
mcpu supported, march not supported
machine_args = ['-mcpu=neoverse-n2']
mcpu not supported, march supported
machine_args = ['-march=armv9-a']
mcpu not supported, march not supported
machine_args = ['-march=armv8.6-a']
Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
config/arm/meson.build | 109 +++++++++++++++++++++++++----------------
1 file changed, 67 insertions(+), 42 deletions(-)
Comments
+Cc
On 2024/1/21 5:36 PM, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> The compiler options march and mtune are a subset
> of mcpu and will lead to conflicts if improper march
> is chosen for a given mcpu.
> To avoid conflicts, force part number march when
> mcpu is available and is supported by the compiler.
>
> Example:
> march = armv9-a
> mcpu = neoverse-n2
>
> mcpu supported, march supported
> machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
>
> mcpu supported, march not supported
> machine_args = ['-mcpu=neoverse-n2']
>
> mcpu not supported, march supported
> machine_args = ['-march=armv9-a']
>
> mcpu not supported, march not supported
> machine_args = ['-march=armv8.6-a']
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
> config/arm/meson.build | 109 +++++++++++++++++++++++++----------------
> 1 file changed, 67 insertions(+), 42 deletions(-)
>
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 36f21d2259..8c8cfccca0 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -58,18 +58,18 @@ implementer_generic = {
> }
>
> part_number_config_arm = {
> - '0xd03': {'compiler_options': ['-mcpu=cortex-a53']},
> - '0xd04': {'compiler_options': ['-mcpu=cortex-a35']},
> - '0xd05': {'compiler_options': ['-mcpu=cortex-a55']},
> - '0xd07': {'compiler_options': ['-mcpu=cortex-a57']},
> - '0xd08': {'compiler_options': ['-mcpu=cortex-a72']},
> - '0xd09': {'compiler_options': ['-mcpu=cortex-a73']},
> - '0xd0a': {'compiler_options': ['-mcpu=cortex-a75']},
> - '0xd0b': {'compiler_options': ['-mcpu=cortex-a76']},
> + '0xd03': {'mcpu': 'cortex-a53'},
> + '0xd04': {'mcpu': 'cortex-a35'},
> + '0xd05': {'mcpu': 'cortex-a55'},
> + '0xd07': {'mcpu': 'cortex-a57'},
> + '0xd08': {'mcpu': 'cortex-a72'},
> + '0xd09': {'mcpu': 'cortex-a73'},
> + '0xd0a': {'mcpu': 'cortex-a75'},
> + '0xd0b': {'mcpu': 'cortex-a76'},
> '0xd0c': {
> 'march': 'armv8.2-a',
> 'march_features': ['crypto', 'rcpc'],
> - 'compiler_options': ['-mcpu=neoverse-n1'],
> + 'mcpu': 'neoverse-n1',
> 'flags': [
> ['RTE_MACHINE', '"neoverse-n1"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -81,7 +81,7 @@ part_number_config_arm = {
> '0xd40': {
> 'march': 'armv8.4-a',
> 'march_features': ['sve'],
> - 'compiler_options': ['-mcpu=neoverse-v1'],
> + 'mcpu': 'neoverse-v1',
> 'flags': [
> ['RTE_MACHINE', '"neoverse-v1"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -92,8 +92,9 @@ part_number_config_arm = {
> 'march': 'armv8.4-a',
> },
> '0xd49': {
> + 'march': 'armv9-a',
> 'march_features': ['sve2'],
> - 'compiler_options': ['-mcpu=neoverse-n2'],
> + 'mcpu': 'neoverse-n2',
> 'flags': [
> ['RTE_MACHINE', '"neoverse-n2"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -127,21 +128,22 @@ implementer_cavium = {
> ],
> 'part_number_config': {
> '0xa1': {
> - 'compiler_options': ['-mcpu=thunderxt88'],
> + 'mcpu': 'thunderxt88',
> 'flags': flags_part_number_thunderx
> },
> '0xa2': {
> - 'compiler_options': ['-mcpu=thunderxt81'],
> + 'mcpu': 'thunderxt81',
> 'flags': flags_part_number_thunderx
> },
> '0xa3': {
> - 'compiler_options': ['-march=armv8-a+crc', '-mcpu=thunderxt83'],
> + 'mcpu': 'thunderxt83',
> + 'compiler_options': ['-march=armv8-a+crc'],
> 'flags': flags_part_number_thunderx
> },
> '0xaf': {
> 'march': 'armv8.1-a',
> 'march_features': ['crc', 'crypto'],
> - 'compiler_options': ['-mcpu=thunderx2t99'],
> + 'mcpu': 'thunderx2t99',
> 'flags': [
> ['RTE_MACHINE', '"thunderx2"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -153,7 +155,7 @@ implementer_cavium = {
> '0xb2': {
> 'march': 'armv8.2-a',
> 'march_features': ['crc', 'crypto', 'lse'],
> - 'compiler_options': ['-mcpu=octeontx2'],
> + 'mcpu': 'octeontx2',
> 'flags': [
> ['RTE_MACHINE', '"cn9k"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -176,7 +178,7 @@ implementer_ampere = {
> '0x0': {
> 'march': 'armv8-a',
> 'march_features': ['crc', 'crypto'],
> - 'compiler_options': ['-mtune=emag'],
> + 'mcpu': 'emag',
> 'flags': [
> ['RTE_MACHINE', '"eMAG"'],
> ['RTE_MAX_LCORE', 32],
> @@ -186,7 +188,7 @@ implementer_ampere = {
> '0xac3': {
> 'march': 'armv8.6-a',
> 'march_features': ['crc', 'crypto'],
> - 'compiler_options': ['-mcpu=ampere1'],
> + 'mcpu': 'ampere1',
> 'flags': [
> ['RTE_MACHINE', '"AmpereOne"'],
> ['RTE_MAX_LCORE', 320],
> @@ -206,7 +208,7 @@ implementer_hisilicon = {
> '0xd01': {
> 'march': 'armv8.2-a',
> 'march_features': ['crypto'],
> - 'compiler_options': ['-mtune=tsv110'],
> + 'mcpu': 'tsv110',
> 'flags': [
> ['RTE_MACHINE', '"Kunpeng 920"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -695,11 +697,23 @@ if update_flags
>
> machine_args = [] # Clear previous machine args
>
> + candidate_mcpu = ''
> + support_mcpu = false
> + if part_number_config.has_key('mcpu')
> + mcpu = part_number_config['mcpu']
> + if (cc.has_argument('-mcpu=' + mcpu))
> + candidate_mcpu = mcpu
> + support_mcpu = true
> + endif
> + endif
> +
> # probe supported archs and their features
> candidate_march = ''
> if part_number_config.has_key('march')
> - if part_number_config.get('force_march', false)
> - candidate_march = part_number_config['march']
> + if part_number_config.get('force_march', false) or support_mcpu
> + if cc.has_argument('-march=' + part_number_config['march'])
> + candidate_march = part_number_config['march']
> + endif
> else
> supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a', 'armv8.3-a',
> 'armv8.2-a', 'armv8.1-a', 'armv8-a']
> @@ -717,32 +731,43 @@ if update_flags
> endif
> endforeach
> endif
> - if candidate_march == ''
> - error('No suitable armv8 march version found.')
> - endif
> +
> 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))
> + warning('Configuration march version is @0@, not supported.'
> + .format(part_number_config['march']))
> + if candidate_march != ''
> + warning('Using march version @0@.'.format(candidate_march))
> + endif
> endif
> - candidate_march = '-march=' + candidate_march
>
> - march_features = []
> - if part_number_config.has_key('march_features')
> - march_features += part_number_config['march_features']
> + if candidate_march == '' and candidate_mcpu == ''
> + error('No suitable ARM march/mcpu version found.')
> endif
> - if soc_config.has_key('extra_march_features')
> - march_features += soc_config['extra_march_features']
> - endif
> - foreach feature: march_features
> - if cc.has_argument('+'.join([candidate_march, feature]))
> - candidate_march = '+'.join([candidate_march, feature])
> - else
> - warning('The compiler does not support feature @0@'
> - .format(feature))
> +
> + if candidate_march != ''
> + candidate_march = '-march=' + candidate_march
> + march_features = []
> + if part_number_config.has_key('march_features')
> + march_features += part_number_config['march_features']
> endif
> - endforeach
> - machine_args += candidate_march
> + if soc_config.has_key('extra_march_features')
> + march_features += soc_config['extra_march_features']
> + endif
> + foreach feature: march_features
> + if cc.has_argument('+'.join([candidate_march, feature]))
> + candidate_march = '+'.join([candidate_march, feature])
> + else
> + warning('The compiler does not support feature @0@'
> + .format(feature))
> + endif
> + endforeach
> + machine_args += candidate_march
> + endif
> + endif
> +
> + if candidate_mcpu != ''
> + candidate_mcpu = '-mcpu=' + candidate_mcpu
> + machine_args += candidate_mcpu
> endif
>
> # apply supported compiler options
On Sun, Jan 21, 2024 at 10:37 AM <pbhagavatula@marvell.com> wrote:
>
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> The compiler options march and mtune are a subset
> of mcpu and will lead to conflicts if improper march
> is chosen for a given mcpu.
> To avoid conflicts, force part number march when
> mcpu is available and is supported by the compiler.
>
> Example:
> march = armv9-a
> mcpu = neoverse-n2
>
> mcpu supported, march supported
> machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
>
> mcpu supported, march not supported
> machine_args = ['-mcpu=neoverse-n2']
>
> mcpu not supported, march supported
> machine_args = ['-march=armv9-a']
>
> mcpu not supported, march not supported
> machine_args = ['-march=armv8.6-a']
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
> config/arm/meson.build | 109 +++++++++++++++++++++++++----------------
> 1 file changed, 67 insertions(+), 42 deletions(-)
>
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 36f21d2259..8c8cfccca0 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
<snip>
> @@ -127,21 +128,22 @@ implementer_cavium = {
> ],
> 'part_number_config': {
> '0xa1': {
> - 'compiler_options': ['-mcpu=thunderxt88'],
> + 'mcpu': 'thunderxt88',
> 'flags': flags_part_number_thunderx
> },
> '0xa2': {
> - 'compiler_options': ['-mcpu=thunderxt81'],
> + 'mcpu': 'thunderxt81',
> 'flags': flags_part_number_thunderx
> },
> '0xa3': {
> - 'compiler_options': ['-march=armv8-a+crc', '-mcpu=thunderxt83'],
> + 'mcpu': 'thunderxt83',
> + 'compiler_options': ['-march=armv8-a+crc'],
Let's unify this with the rest and specify 'march': 'armv8-a+crc'
instead of having it under compiler_options.
> 'flags': flags_part_number_thunderx
> },
> '0xaf': {
> 'march': 'armv8.1-a',
> 'march_features': ['crc', 'crypto'],
> - 'compiler_options': ['-mcpu=thunderx2t99'],
> + 'mcpu': 'thunderx2t99',
> 'flags': [
> ['RTE_MACHINE', '"thunderx2"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -153,7 +155,7 @@ implementer_cavium = {
> '0xb2': {
> 'march': 'armv8.2-a',
> 'march_features': ['crc', 'crypto', 'lse'],
> - 'compiler_options': ['-mcpu=octeontx2'],
> + 'mcpu': 'octeontx2',
> 'flags': [
> ['RTE_MACHINE', '"cn9k"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -176,7 +178,7 @@ implementer_ampere = {
> '0x0': {
> 'march': 'armv8-a',
> 'march_features': ['crc', 'crypto'],
> - 'compiler_options': ['-mtune=emag'],
> + 'mcpu': 'emag',
We're changing mtune to mcpu, is this equivalent?
> 'flags': [
> ['RTE_MACHINE', '"eMAG"'],
> ['RTE_MAX_LCORE', 32],
> @@ -186,7 +188,7 @@ implementer_ampere = {
> '0xac3': {
> 'march': 'armv8.6-a',
> 'march_features': ['crc', 'crypto'],
> - 'compiler_options': ['-mcpu=ampere1'],
> + 'mcpu': 'ampere1',
> 'flags': [
> ['RTE_MACHINE', '"AmpereOne"'],
> ['RTE_MAX_LCORE', 320],
> @@ -206,7 +208,7 @@ implementer_hisilicon = {
> '0xd01': {
> 'march': 'armv8.2-a',
> 'march_features': ['crypto'],
> - 'compiler_options': ['-mtune=tsv110'],
> + 'mcpu': 'tsv110',
> 'flags': [
> ['RTE_MACHINE', '"Kunpeng 920"'],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> @@ -695,11 +697,23 @@ if update_flags
>
> machine_args = [] # Clear previous machine args
>
> + candidate_mcpu = ''
> + support_mcpu = false
> + if part_number_config.has_key('mcpu')
> + mcpu = part_number_config['mcpu']
> + if (cc.has_argument('-mcpu=' + mcpu))
> + candidate_mcpu = mcpu
> + support_mcpu = true
> + endif
> + endif
> +
> # probe supported archs and their features
> candidate_march = ''
> if part_number_config.has_key('march')
> - if part_number_config.get('force_march', false)
> - candidate_march = part_number_config['march']
> + if part_number_config.get('force_march', false) or support_mcpu
Instead of using the extra "support_mcpu" variable, we could do the
same check as with candidate march (if candidate_mcpu != '', which we
actually do below in the last lines of the patch).
If I understand the logic correctly, we don't want to do the march
fallback if mcpu is specified - either the march works with the given
mcpu or we do without it (because we don't actually need it with
mcpu). Is that correct?
> + if cc.has_argument('-march=' + part_number_config['march'])
Now that we've added mcpu into the mix, is this still the right
condition? Can the below happen?
This check finds that machine_args = ['-march=armv9-a'] is supported.
But taken together with mcpu (machine_args = ['-mcpu=neoverse-n2',
'-march=armv9-a']), it is not supported? In this case we'll end up
with invalid configuration.
> + candidate_march = part_number_config['march']
> + endif
> else
> supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a', 'armv8.3-a',
> 'armv8.2-a', 'armv8.1-a', 'armv8-a']
> On Sun, Jan 21, 2024 at 10:37 AM <pbhagavatula@marvell.com> wrote:
> >
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > The compiler options march and mtune are a subset
> > of mcpu and will lead to conflicts if improper march
> > is chosen for a given mcpu.
> > To avoid conflicts, force part number march when
> > mcpu is available and is supported by the compiler.
> >
> > Example:
> > march = armv9-a
> > mcpu = neoverse-n2
> >
> > mcpu supported, march supported
> > machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
> >
> > mcpu supported, march not supported
> > machine_args = ['-mcpu=neoverse-n2']
> >
> > mcpu not supported, march supported
> > machine_args = ['-march=armv9-a']
> >
> > mcpu not supported, march not supported
> > machine_args = ['-march=armv8.6-a']
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> > config/arm/meson.build | 109 +++++++++++++++++++++++++--------------
> --
> > 1 file changed, 67 insertions(+), 42 deletions(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > index 36f21d2259..8c8cfccca0 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> <snip>
> > @@ -127,21 +128,22 @@ implementer_cavium = {
> > ],
> > 'part_number_config': {
> > '0xa1': {
> > - 'compiler_options': ['-mcpu=thunderxt88'],
> > + 'mcpu': 'thunderxt88',
> > 'flags': flags_part_number_thunderx
> > },
> > '0xa2': {
> > - 'compiler_options': ['-mcpu=thunderxt81'],
> > + 'mcpu': 'thunderxt81',
> > 'flags': flags_part_number_thunderx
> > },
> > '0xa3': {
> > - 'compiler_options': ['-march=armv8-a+crc', '-mcpu=thunderxt83'],
> > + 'mcpu': 'thunderxt83',
> > + 'compiler_options': ['-march=armv8-a+crc'],
>
> Let's unify this with the rest and specify 'march': 'armv8-a+crc'
> instead of having it under compiler_options.
Ack.
>
> > 'flags': flags_part_number_thunderx
> > },
> > '0xaf': {
> > 'march': 'armv8.1-a',
> > 'march_features': ['crc', 'crypto'],
> > - 'compiler_options': ['-mcpu=thunderx2t99'],
> > + 'mcpu': 'thunderx2t99',
> > 'flags': [
> > ['RTE_MACHINE', '"thunderx2"'],
> > ['RTE_ARM_FEATURE_ATOMICS', true],
> > @@ -153,7 +155,7 @@ implementer_cavium = {
> > '0xb2': {
> > 'march': 'armv8.2-a',
> > 'march_features': ['crc', 'crypto', 'lse'],
> > - 'compiler_options': ['-mcpu=octeontx2'],
> > + 'mcpu': 'octeontx2',
> > 'flags': [
> > ['RTE_MACHINE', '"cn9k"'],
> > ['RTE_ARM_FEATURE_ATOMICS', true],
> > @@ -176,7 +178,7 @@ implementer_ampere = {
> > '0x0': {
> > 'march': 'armv8-a',
> > 'march_features': ['crc', 'crypto'],
> > - 'compiler_options': ['-mtune=emag'],
> > + 'mcpu': 'emag',
>
> We're changing mtune to mcpu, is this equivalent?
>
Both march and mtune are a subset of mcpu.
> > 'flags': [
> > ['RTE_MACHINE', '"eMAG"'],
> > ['RTE_MAX_LCORE', 32],
> > @@ -186,7 +188,7 @@ implementer_ampere = {
> > '0xac3': {
> > 'march': 'armv8.6-a',
> > 'march_features': ['crc', 'crypto'],
> > - 'compiler_options': ['-mcpu=ampere1'],
> > + 'mcpu': 'ampere1',
> > 'flags': [
> > ['RTE_MACHINE', '"AmpereOne"'],
> > ['RTE_MAX_LCORE', 320],
> > @@ -206,7 +208,7 @@ implementer_hisilicon = {
> > '0xd01': {
> > 'march': 'armv8.2-a',
> > 'march_features': ['crypto'],
> > - 'compiler_options': ['-mtune=tsv110'],
> > + 'mcpu': 'tsv110',
> > 'flags': [
> > ['RTE_MACHINE', '"Kunpeng 920"'],
> > ['RTE_ARM_FEATURE_ATOMICS', true],
> > @@ -695,11 +697,23 @@ if update_flags
> >
> > machine_args = [] # Clear previous machine args
> >
> > + candidate_mcpu = ''
> > + support_mcpu = false
> > + if part_number_config.has_key('mcpu')
> > + mcpu = part_number_config['mcpu']
> > + if (cc.has_argument('-mcpu=' + mcpu))
> > + candidate_mcpu = mcpu
> > + support_mcpu = true
> > + endif
> > + endif
> > +
> > # probe supported archs and their features
> > candidate_march = ''
> > if part_number_config.has_key('march')
> > - if part_number_config.get('force_march', false)
> > - candidate_march = part_number_config['march']
> > + if part_number_config.get('force_march', false) or support_mcpu
>
> Instead of using the extra "support_mcpu" variable, we could do the
> same check as with candidate march (if candidate_mcpu != '', which we
> actually do below in the last lines of the patch).
>
Ack.
> If I understand the logic correctly, we don't want to do the march
> fallback if mcpu is specified - either the march works with the given
> mcpu or we do without it (because we don't actually need it with
> mcpu). Is that correct?
>
Yes, but still exact march defined in part_number_config should be present for setting extra_march_features.
specially for expressing crypto support.
> > + if cc.has_argument('-march=' + part_number_config['march'])
>
> Now that we've added mcpu into the mix, is this still the right
> condition? Can the below happen?
>
> This check finds that machine_args = ['-march=armv9-a'] is supported.
>
> But taken together with mcpu (machine_args = ['-mcpu=neoverse-n2',
> '-march=armv9-a']), it is not supported? In this case we'll end up
> with invalid configuration.
This is the only correct option and evolves into -march=armv9-a+sve2+crypto for cn10k
whereas other neoverse-n2 might only have -march=armv9-a+sve2.
Example:
Good:
#aarch64-linux-gnu-gcc -march=armv9-a+sve2+crypto -mcpu=neoverse-n2 shrn.c
#aarch64-linux-gnu-gcc -march=armv9-a+sve2 -mcpu=neoverse-n2 shrn.c
#aarch64-linux-gnu-gcc -march=armv9-a -mcpu=neoverse-n2 shrn.c
Bad:
#aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8-a shrn.c
cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8-a' switch
#aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.1-a shrn.c
cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.1-a' switch
#aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.2-a shrn.c
cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.2-a' switch
#aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.3-a shrn.c
cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.3-a' switch
#aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.4-a shrn.c
cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.4-a' switch
#aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.5-a shrn.c
cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.5-a' switch
#aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c
cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.6-a' switch
#aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.7-a shrn.c
cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.7-a' switch
>
> > + candidate_march = part_number_config['march']
> > + endif
> > else
> > supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a', 'armv8.3-
> a',
> > 'armv8.2-a', 'armv8.1-a', 'armv8-a']
On Mon, Jan 22, 2024 at 12:54 PM Pavan Nikhilesh Bhagavatula
<pbhagavatula@marvell.com> wrote:
>
> > On Sun, Jan 21, 2024 at 10:37 AM <pbhagavatula@marvell.com> wrote:
> > >
> > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >
> > > The compiler options march and mtune are a subset
> > > of mcpu and will lead to conflicts if improper march
> > > is chosen for a given mcpu.
> > > To avoid conflicts, force part number march when
> > > mcpu is available and is supported by the compiler.
> > >
> > > Example:
> > > march = armv9-a
> > > mcpu = neoverse-n2
> > >
> > > mcpu supported, march supported
> > > machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
> > >
> > > mcpu supported, march not supported
> > > machine_args = ['-mcpu=neoverse-n2']
> > >
> > > mcpu not supported, march supported
> > > machine_args = ['-march=armv9-a']
> > >
> > > mcpu not supported, march not supported
> > > machine_args = ['-march=armv8.6-a']
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > ---
> > > config/arm/meson.build | 109 +++++++++++++++++++++++++--------------
> > --
> > > 1 file changed, 67 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > index 36f21d2259..8c8cfccca0 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > <snip>
> > > @@ -127,21 +128,22 @@ implementer_cavium = {
> > > ],
> > > 'part_number_config': {
> > > '0xa1': {
> > > - 'compiler_options': ['-mcpu=thunderxt88'],
> > > + 'mcpu': 'thunderxt88',
> > > 'flags': flags_part_number_thunderx
> > > },
> > > '0xa2': {
> > > - 'compiler_options': ['-mcpu=thunderxt81'],
> > > + 'mcpu': 'thunderxt81',
> > > 'flags': flags_part_number_thunderx
> > > },
> > > '0xa3': {
> > > - 'compiler_options': ['-march=armv8-a+crc', '-mcpu=thunderxt83'],
> > > + 'mcpu': 'thunderxt83',
> > > + 'compiler_options': ['-march=armv8-a+crc'],
> >
> > Let's unify this with the rest and specify 'march': 'armv8-a+crc'
> > instead of having it under compiler_options.
>
> Ack.
>
> >
> > > 'flags': flags_part_number_thunderx
> > > },
> > > '0xaf': {
> > > 'march': 'armv8.1-a',
> > > 'march_features': ['crc', 'crypto'],
> > > - 'compiler_options': ['-mcpu=thunderx2t99'],
> > > + 'mcpu': 'thunderx2t99',
> > > 'flags': [
> > > ['RTE_MACHINE', '"thunderx2"'],
> > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > @@ -153,7 +155,7 @@ implementer_cavium = {
> > > '0xb2': {
> > > 'march': 'armv8.2-a',
> > > 'march_features': ['crc', 'crypto', 'lse'],
> > > - 'compiler_options': ['-mcpu=octeontx2'],
> > > + 'mcpu': 'octeontx2',
> > > 'flags': [
> > > ['RTE_MACHINE', '"cn9k"'],
> > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > @@ -176,7 +178,7 @@ implementer_ampere = {
> > > '0x0': {
> > > 'march': 'armv8-a',
> > > 'march_features': ['crc', 'crypto'],
> > > - 'compiler_options': ['-mtune=emag'],
> > > + 'mcpu': 'emag',
> >
> > We're changing mtune to mcpu, is this equivalent?
> >
>
> Both march and mtune are a subset of mcpu.
>
Sure, but we replaced '-mtune=emag' with '-mcpu=emag'. Are these two
builds going to be different or the same?
> > > 'flags': [
> > > ['RTE_MACHINE', '"eMAG"'],
> > > ['RTE_MAX_LCORE', 32],
> > > @@ -186,7 +188,7 @@ implementer_ampere = {
> > > '0xac3': {
> > > 'march': 'armv8.6-a',
> > > 'march_features': ['crc', 'crypto'],
> > > - 'compiler_options': ['-mcpu=ampere1'],
> > > + 'mcpu': 'ampere1',
> > > 'flags': [
> > > ['RTE_MACHINE', '"AmpereOne"'],
> > > ['RTE_MAX_LCORE', 320],
> > > @@ -206,7 +208,7 @@ implementer_hisilicon = {
> > > '0xd01': {
> > > 'march': 'armv8.2-a',
> > > 'march_features': ['crypto'],
> > > - 'compiler_options': ['-mtune=tsv110'],
> > > + 'mcpu': 'tsv110',
> > > 'flags': [
> > > ['RTE_MACHINE', '"Kunpeng 920"'],
> > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > @@ -695,11 +697,23 @@ if update_flags
> > >
> > > machine_args = [] # Clear previous machine args
> > >
> > > + candidate_mcpu = ''
> > > + support_mcpu = false
> > > + if part_number_config.has_key('mcpu')
> > > + mcpu = part_number_config['mcpu']
> > > + if (cc.has_argument('-mcpu=' + mcpu))
> > > + candidate_mcpu = mcpu
> > > + support_mcpu = true
> > > + endif
> > > + endif
> > > +
> > > # probe supported archs and their features
> > > candidate_march = ''
> > > if part_number_config.has_key('march')
> > > - if part_number_config.get('force_march', false)
> > > - candidate_march = part_number_config['march']
> > > + if part_number_config.get('force_march', false) or support_mcpu
> >
> > Instead of using the extra "support_mcpu" variable, we could do the
> > same check as with candidate march (if candidate_mcpu != '', which we
> > actually do below in the last lines of the patch).
> >
>
> Ack.
>
> > If I understand the logic correctly, we don't want to do the march
> > fallback if mcpu is specified - either the march works with the given
> > mcpu or we do without it (because we don't actually need it with
> > mcpu). Is that correct?
> >
>
> Yes, but still exact march defined in part_number_config should be present for setting extra_march_features.
> specially for expressing crypto support.
>
Ok, thanks.
> > > + if cc.has_argument('-march=' + part_number_config['march'])
> >
> > Now that we've added mcpu into the mix, is this still the right
> > condition? Can the below happen?
> >
> > This check finds that machine_args = ['-march=armv9-a'] is supported.
> >
> > But taken together with mcpu (machine_args = ['-mcpu=neoverse-n2',
> > '-march=armv9-a']), it is not supported? In this case we'll end up
> > with invalid configuration.
>
> This is the only correct option and evolves into -march=armv9-a+sve2+crypto for cn10k
> whereas other neoverse-n2 might only have -march=armv9-a+sve2.
>
Maybe I should rephrase my question a bit:
The correct options are ['-mcpu=neoverse-n2', '-march=armv9-a']. Is it
possible that the compiler will say:
['-mcpu=neoverse-n2', '-march=armv9-a'] is supported
['-mcpu=neoverse-n2'] is supported
['-march=armv9-a'] is not supported
So basically the question is are we risking that the compiler will say
it supports both options only when both are passed while also saying
it doesn't support one or both of them when checked alone. We've seen
this behavior with newer compilers in aarch32 builds
(-march=armv8-a+simd -mfpu=auto are supported when both are passed,
but -march=armv8-a is not supported alone), so I wanted to be sure.
> Example:
>
> Good:
> #aarch64-linux-gnu-gcc -march=armv9-a+sve2+crypto -mcpu=neoverse-n2 shrn.c
> #aarch64-linux-gnu-gcc -march=armv9-a+sve2 -mcpu=neoverse-n2 shrn.c
> #aarch64-linux-gnu-gcc -march=armv9-a -mcpu=neoverse-n2 shrn.c
>
> Bad:
> #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8-a shrn.c
> cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8-a' switch
> #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.1-a shrn.c
> cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.1-a' switch
> #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.2-a shrn.c
> cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.2-a' switch
> #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.3-a shrn.c
> cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.3-a' switch
> #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.4-a shrn.c
> cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.4-a' switch
> #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.5-a shrn.c
> cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.5-a' switch
> #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c
> cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.6-a' switch
> #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.7-a shrn.c
> cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.7-a' switch
>
> >
> > > + candidate_march = part_number_config['march']
> > > + endif
> > > else
> > > supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a', 'armv8.3-
> > a',
> > > 'armv8.2-a', 'armv8.1-a', 'armv8-a']
> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Monday, January 22, 2024 9:57 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Cc: Jerin Jacob <jerinj@marvell.com>; Ruifeng.Wang@arm.com;
> nd@arm.com; Bruce Richardson <bruce.richardson@intel.com>;
> dev@dpdk.org
> Subject: Re: [EXT] Re: [PATCH 1/2] config/arm: avoid mcpu and march
> conflicts
>
> On Mon, Jan 22, 2024 at 12:54 PM Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com> wrote:
> >
> > > On Sun, Jan 21, 2024 at 10:37 AM <pbhagavatula@marvell.com> wrote:
> > > >
> > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > >
> > > > The compiler options march and mtune are a subset
> > > > of mcpu and will lead to conflicts if improper march
> > > > is chosen for a given mcpu.
> > > > To avoid conflicts, force part number march when
> > > > mcpu is available and is supported by the compiler.
> > > >
> > > > Example:
> > > > march = armv9-a
> > > > mcpu = neoverse-n2
> > > >
> > > > mcpu supported, march supported
> > > > machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
> > > >
> > > > mcpu supported, march not supported
> > > > machine_args = ['-mcpu=neoverse-n2']
> > > >
> > > > mcpu not supported, march supported
> > > > machine_args = ['-march=armv9-a']
> > > >
> > > > mcpu not supported, march not supported
> > > > machine_args = ['-march=armv8.6-a']
> > > >
> > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > ---
> > > > config/arm/meson.build | 109 +++++++++++++++++++++++++---------
> -----
> > > --
> > > > 1 file changed, 67 insertions(+), 42 deletions(-)
> > > >
> > > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > > index 36f21d2259..8c8cfccca0 100644
> > > > --- a/config/arm/meson.build
> > > > +++ b/config/arm/meson.build
> > > <snip>
> > > > @@ -127,21 +128,22 @@ implementer_cavium = {
> > > > ],
> > > > 'part_number_config': {
> > > > '0xa1': {
> > > > - 'compiler_options': ['-mcpu=thunderxt88'],
> > > > + 'mcpu': 'thunderxt88',
> > > > 'flags': flags_part_number_thunderx
> > > > },
> > > > '0xa2': {
> > > > - 'compiler_options': ['-mcpu=thunderxt81'],
> > > > + 'mcpu': 'thunderxt81',
> > > > 'flags': flags_part_number_thunderx
> > > > },
> > > > '0xa3': {
> > > > - 'compiler_options': ['-march=armv8-a+crc', '-
> mcpu=thunderxt83'],
> > > > + 'mcpu': 'thunderxt83',
> > > > + 'compiler_options': ['-march=armv8-a+crc'],
> > >
> > > Let's unify this with the rest and specify 'march': 'armv8-a+crc'
> > > instead of having it under compiler_options.
> >
> > Ack.
> >
> > >
> > > > 'flags': flags_part_number_thunderx
> > > > },
> > > > '0xaf': {
> > > > 'march': 'armv8.1-a',
> > > > 'march_features': ['crc', 'crypto'],
> > > > - 'compiler_options': ['-mcpu=thunderx2t99'],
> > > > + 'mcpu': 'thunderx2t99',
> > > > 'flags': [
> > > > ['RTE_MACHINE', '"thunderx2"'],
> > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > @@ -153,7 +155,7 @@ implementer_cavium = {
> > > > '0xb2': {
> > > > 'march': 'armv8.2-a',
> > > > 'march_features': ['crc', 'crypto', 'lse'],
> > > > - 'compiler_options': ['-mcpu=octeontx2'],
> > > > + 'mcpu': 'octeontx2',
> > > > 'flags': [
> > > > ['RTE_MACHINE', '"cn9k"'],
> > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > @@ -176,7 +178,7 @@ implementer_ampere = {
> > > > '0x0': {
> > > > 'march': 'armv8-a',
> > > > 'march_features': ['crc', 'crypto'],
> > > > - 'compiler_options': ['-mtune=emag'],
> > > > + 'mcpu': 'emag',
> > >
> > > We're changing mtune to mcpu, is this equivalent?
> > >
> >
> > Both march and mtune are a subset of mcpu.
> >
>
> Sure, but we replaced '-mtune=emag' with '-mcpu=emag'. Are these two
> builds going to be different or the same?
>
Yeah, I believe both are same.
> > > > 'flags': [
> > > > ['RTE_MACHINE', '"eMAG"'],
> > > > ['RTE_MAX_LCORE', 32],
> > > > @@ -186,7 +188,7 @@ implementer_ampere = {
> > > > '0xac3': {
> > > > 'march': 'armv8.6-a',
> > > > 'march_features': ['crc', 'crypto'],
> > > > - 'compiler_options': ['-mcpu=ampere1'],
> > > > + 'mcpu': 'ampere1',
> > > > 'flags': [
> > > > ['RTE_MACHINE', '"AmpereOne"'],
> > > > ['RTE_MAX_LCORE', 320],
> > > > @@ -206,7 +208,7 @@ implementer_hisilicon = {
> > > > '0xd01': {
> > > > 'march': 'armv8.2-a',
> > > > 'march_features': ['crypto'],
> > > > - 'compiler_options': ['-mtune=tsv110'],
> > > > + 'mcpu': 'tsv110',
> > > > 'flags': [
> > > > ['RTE_MACHINE', '"Kunpeng 920"'],
> > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > @@ -695,11 +697,23 @@ if update_flags
> > > >
> > > > machine_args = [] # Clear previous machine args
> > > >
> > > > + candidate_mcpu = ''
> > > > + support_mcpu = false
> > > > + if part_number_config.has_key('mcpu')
> > > > + mcpu = part_number_config['mcpu']
> > > > + if (cc.has_argument('-mcpu=' + mcpu))
> > > > + candidate_mcpu = mcpu
> > > > + support_mcpu = true
> > > > + endif
> > > > + endif
> > > > +
> > > > # probe supported archs and their features
> > > > candidate_march = ''
> > > > if part_number_config.has_key('march')
> > > > - if part_number_config.get('force_march', false)
> > > > - candidate_march = part_number_config['march']
> > > > + if part_number_config.get('force_march', false) or support_mcpu
> > >
> > > Instead of using the extra "support_mcpu" variable, we could do the
> > > same check as with candidate march (if candidate_mcpu != '', which we
> > > actually do below in the last lines of the patch).
> > >
> >
> > Ack.
> >
> > > If I understand the logic correctly, we don't want to do the march
> > > fallback if mcpu is specified - either the march works with the given
> > > mcpu or we do without it (because we don't actually need it with
> > > mcpu). Is that correct?
> > >
> >
> > Yes, but still exact march defined in part_number_config should be present
> for setting extra_march_features.
> > specially for expressing crypto support.
> >
>
> Ok, thanks.
>
> > > > + if cc.has_argument('-march=' + part_number_config['march'])
> > >
> > > Now that we've added mcpu into the mix, is this still the right
> > > condition? Can the below happen?
> > >
> > > This check finds that machine_args = ['-march=armv9-a'] is supported.
> > >
> > > But taken together with mcpu (machine_args = ['-mcpu=neoverse-n2',
> > > '-march=armv9-a']), it is not supported? In this case we'll end up
> > > with invalid configuration.
> >
> > This is the only correct option and evolves into -march=armv9-
> a+sve2+crypto for cn10k
> > whereas other neoverse-n2 might only have -march=armv9-a+sve2.
> >
>
> Maybe I should rephrase my question a bit:
>
> The correct options are ['-mcpu=neoverse-n2', '-march=armv9-a']. Is it
> possible that the compiler will say:
>
> ['-mcpu=neoverse-n2', '-march=armv9-a'] is supported
> ['-mcpu=neoverse-n2'] is supported
> ['-march=armv9-a'] is not supported
>
Yes, this is possible.
> So basically the question is are we risking that the compiler will say
> it supports both options only when both are passed while also saying
> it doesn't support one or both of them when checked alone. We've seen
> this behavior with newer compilers in aarch32 builds
> (-march=armv8-a+simd -mfpu=auto are supported when both are passed,
> but -march=armv8-a is not supported alone), so I wanted to be sure.
>
> > Example:
> >
> > Good:
> > #aarch64-linux-gnu-gcc -march=armv9-a+sve2+crypto -mcpu=neoverse-n2
> shrn.c
> > #aarch64-linux-gnu-gcc -march=armv9-a+sve2 -mcpu=neoverse-n2 shrn.c
> > #aarch64-linux-gnu-gcc -march=armv9-a -mcpu=neoverse-n2 shrn.c
> >
> > Bad:
> > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8-a shrn.c
> > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8-a'
> switch
> > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.1-a shrn.c
> > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.1-a'
> switch
> > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.2-a shrn.c
> > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.2-a'
> switch
> > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.3-a shrn.c
> > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.3-a'
> switch
> > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.4-a shrn.c
> > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.4-a'
> switch
> > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.5-a shrn.c
> > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.5-a'
> switch
> > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c
> > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.6-a'
> switch
> > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.7-a shrn.c
> > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.7-a'
> switch
> >
> > >
> > > > + candidate_march = part_number_config['march']
> > > > + endif
> > > > else
> > > > supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a',
> 'armv8.3-
> > > a',
> > > > 'armv8.2-a', 'armv8.1-a', 'armv8-a']
On Wed, Jan 24, 2024 at 4:22 PM Pavan Nikhilesh Bhagavatula
<pbhagavatula@marvell.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Sent: Monday, January 22, 2024 9:57 PM
> > To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > Cc: Jerin Jacob <jerinj@marvell.com>; Ruifeng.Wang@arm.com;
> > nd@arm.com; Bruce Richardson <bruce.richardson@intel.com>;
> > dev@dpdk.org
> > Subject: Re: [EXT] Re: [PATCH 1/2] config/arm: avoid mcpu and march
> > conflicts
> >
> > On Mon, Jan 22, 2024 at 12:54 PM Pavan Nikhilesh Bhagavatula
> > <pbhagavatula@marvell.com> wrote:
> > >
> > > > On Sun, Jan 21, 2024 at 10:37 AM <pbhagavatula@marvell.com> wrote:
> > > > >
> > > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > >
> > > > > The compiler options march and mtune are a subset
> > > > > of mcpu and will lead to conflicts if improper march
> > > > > is chosen for a given mcpu.
> > > > > To avoid conflicts, force part number march when
> > > > > mcpu is available and is supported by the compiler.
> > > > >
> > > > > Example:
> > > > > march = armv9-a
> > > > > mcpu = neoverse-n2
> > > > >
> > > > > mcpu supported, march supported
> > > > > machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
> > > > >
> > > > > mcpu supported, march not supported
> > > > > machine_args = ['-mcpu=neoverse-n2']
> > > > >
> > > > > mcpu not supported, march supported
> > > > > machine_args = ['-march=armv9-a']
> > > > >
> > > > > mcpu not supported, march not supported
> > > > > machine_args = ['-march=armv8.6-a']
> > > > >
> > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > ---
> > > > > config/arm/meson.build | 109 +++++++++++++++++++++++++---------
> > -----
> > > > --
> > > > > 1 file changed, 67 insertions(+), 42 deletions(-)
> > > > >
> > > > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > > > index 36f21d2259..8c8cfccca0 100644
> > > > > --- a/config/arm/meson.build
> > > > > +++ b/config/arm/meson.build
> > > > <snip>
> > > > > @@ -127,21 +128,22 @@ implementer_cavium = {
> > > > > ],
> > > > > 'part_number_config': {
> > > > > '0xa1': {
> > > > > - 'compiler_options': ['-mcpu=thunderxt88'],
> > > > > + 'mcpu': 'thunderxt88',
> > > > > 'flags': flags_part_number_thunderx
> > > > > },
> > > > > '0xa2': {
> > > > > - 'compiler_options': ['-mcpu=thunderxt81'],
> > > > > + 'mcpu': 'thunderxt81',
> > > > > 'flags': flags_part_number_thunderx
> > > > > },
> > > > > '0xa3': {
> > > > > - 'compiler_options': ['-march=armv8-a+crc', '-
> > mcpu=thunderxt83'],
> > > > > + 'mcpu': 'thunderxt83',
> > > > > + 'compiler_options': ['-march=armv8-a+crc'],
> > > >
> > > > Let's unify this with the rest and specify 'march': 'armv8-a+crc'
> > > > instead of having it under compiler_options.
> > >
> > > Ack.
> > >
> > > >
> > > > > 'flags': flags_part_number_thunderx
> > > > > },
> > > > > '0xaf': {
> > > > > 'march': 'armv8.1-a',
> > > > > 'march_features': ['crc', 'crypto'],
> > > > > - 'compiler_options': ['-mcpu=thunderx2t99'],
> > > > > + 'mcpu': 'thunderx2t99',
> > > > > 'flags': [
> > > > > ['RTE_MACHINE', '"thunderx2"'],
> > > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > > @@ -153,7 +155,7 @@ implementer_cavium = {
> > > > > '0xb2': {
> > > > > 'march': 'armv8.2-a',
> > > > > 'march_features': ['crc', 'crypto', 'lse'],
> > > > > - 'compiler_options': ['-mcpu=octeontx2'],
> > > > > + 'mcpu': 'octeontx2',
> > > > > 'flags': [
> > > > > ['RTE_MACHINE', '"cn9k"'],
> > > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > > @@ -176,7 +178,7 @@ implementer_ampere = {
> > > > > '0x0': {
> > > > > 'march': 'armv8-a',
> > > > > 'march_features': ['crc', 'crypto'],
> > > > > - 'compiler_options': ['-mtune=emag'],
> > > > > + 'mcpu': 'emag',
> > > >
> > > > We're changing mtune to mcpu, is this equivalent?
> > > >
> > >
> > > Both march and mtune are a subset of mcpu.
> > >
> >
> > Sure, but we replaced '-mtune=emag' with '-mcpu=emag'. Are these two
> > builds going to be different or the same?
> >
>
> Yeah, I believe both are same.
>
Ok, the change is fine then.
> > > > > 'flags': [
> > > > > ['RTE_MACHINE', '"eMAG"'],
> > > > > ['RTE_MAX_LCORE', 32],
> > > > > @@ -186,7 +188,7 @@ implementer_ampere = {
> > > > > '0xac3': {
> > > > > 'march': 'armv8.6-a',
> > > > > 'march_features': ['crc', 'crypto'],
> > > > > - 'compiler_options': ['-mcpu=ampere1'],
> > > > > + 'mcpu': 'ampere1',
> > > > > 'flags': [
> > > > > ['RTE_MACHINE', '"AmpereOne"'],
> > > > > ['RTE_MAX_LCORE', 320],
> > > > > @@ -206,7 +208,7 @@ implementer_hisilicon = {
> > > > > '0xd01': {
> > > > > 'march': 'armv8.2-a',
> > > > > 'march_features': ['crypto'],
> > > > > - 'compiler_options': ['-mtune=tsv110'],
> > > > > + 'mcpu': 'tsv110',
> > > > > 'flags': [
> > > > > ['RTE_MACHINE', '"Kunpeng 920"'],
> > > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > > @@ -695,11 +697,23 @@ if update_flags
> > > > >
> > > > > machine_args = [] # Clear previous machine args
> > > > >
> > > > > + candidate_mcpu = ''
> > > > > + support_mcpu = false
> > > > > + if part_number_config.has_key('mcpu')
> > > > > + mcpu = part_number_config['mcpu']
> > > > > + if (cc.has_argument('-mcpu=' + mcpu))
> > > > > + candidate_mcpu = mcpu
> > > > > + support_mcpu = true
> > > > > + endif
> > > > > + endif
> > > > > +
> > > > > # probe supported archs and their features
> > > > > candidate_march = ''
> > > > > if part_number_config.has_key('march')
> > > > > - if part_number_config.get('force_march', false)
> > > > > - candidate_march = part_number_config['march']
> > > > > + if part_number_config.get('force_march', false) or support_mcpu
> > > >
> > > > Instead of using the extra "support_mcpu" variable, we could do the
> > > > same check as with candidate march (if candidate_mcpu != '', which we
> > > > actually do below in the last lines of the patch).
> > > >
> > >
> > > Ack.
> > >
> > > > If I understand the logic correctly, we don't want to do the march
> > > > fallback if mcpu is specified - either the march works with the given
> > > > mcpu or we do without it (because we don't actually need it with
> > > > mcpu). Is that correct?
> > > >
> > >
> > > Yes, but still exact march defined in part_number_config should be present
> > for setting extra_march_features.
> > > specially for expressing crypto support.
> > >
> >
> > Ok, thanks.
> >
> > > > > + if cc.has_argument('-march=' + part_number_config['march'])
> > > >
> > > > Now that we've added mcpu into the mix, is this still the right
> > > > condition? Can the below happen?
> > > >
> > > > This check finds that machine_args = ['-march=armv9-a'] is supported.
> > > >
> > > > But taken together with mcpu (machine_args = ['-mcpu=neoverse-n2',
> > > > '-march=armv9-a']), it is not supported? In this case we'll end up
> > > > with invalid configuration.
> > >
> > > This is the only correct option and evolves into -march=armv9-
> > a+sve2+crypto for cn10k
> > > whereas other neoverse-n2 might only have -march=armv9-a+sve2.
> > >
> >
> > Maybe I should rephrase my question a bit:
> >
> > The correct options are ['-mcpu=neoverse-n2', '-march=armv9-a']. Is it
> > possible that the compiler will say:
> >
> > ['-mcpu=neoverse-n2', '-march=armv9-a'] is supported
> > ['-mcpu=neoverse-n2'] is supported
> > ['-march=armv9-a'] is not supported
> >
>
> Yes, this is possible.
>
Ok, do we want to make sure it's possible to configure the options in
this way? But maybe that's out of scope of this patch.
> > So basically the question is are we risking that the compiler will say
> > it supports both options only when both are passed while also saying
> > it doesn't support one or both of them when checked alone. We've seen
> > this behavior with newer compilers in aarch32 builds
> > (-march=armv8-a+simd -mfpu=auto are supported when both are passed,
> > but -march=armv8-a is not supported alone), so I wanted to be sure.
> >
> > > Example:
> > >
> > > Good:
> > > #aarch64-linux-gnu-gcc -march=armv9-a+sve2+crypto -mcpu=neoverse-n2
> > shrn.c
> > > #aarch64-linux-gnu-gcc -march=armv9-a+sve2 -mcpu=neoverse-n2 shrn.c
> > > #aarch64-linux-gnu-gcc -march=armv9-a -mcpu=neoverse-n2 shrn.c
> > >
> > > Bad:
> > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8-a shrn.c
> > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8-a'
> > switch
> > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.1-a shrn.c
> > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.1-a'
> > switch
> > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.2-a shrn.c
> > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.2-a'
> > switch
> > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.3-a shrn.c
> > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.3-a'
> > switch
> > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.4-a shrn.c
> > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.4-a'
> > switch
> > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.5-a shrn.c
> > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.5-a'
> > switch
> > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c
> > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.6-a'
> > switch
> > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.7-a shrn.c
> > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.7-a'
> > switch
> > >
> > > >
> > > > > + candidate_march = part_number_config['march']
> > > > > + endif
> > > > > else
> > > > > supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a',
> > 'armv8.3-
> > > > a',
> > > > > 'armv8.2-a', 'armv8.1-a', 'armv8-a']
> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Monday, January 29, 2024 2:15 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Cc: Jerin Jacob <jerinj@marvell.com>; Ruifeng.Wang@arm.com;
> nd@arm.com; Bruce Richardson <bruce.richardson@intel.com>;
> dev@dpdk.org
> Subject: Re: [EXT] Re: [PATCH 1/2] config/arm: avoid mcpu and march
> conflicts
>
> On Wed, Jan 24, 2024 at 4:22 PM Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > Sent: Monday, January 22, 2024 9:57 PM
> > > To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > > Cc: Jerin Jacob <jerinj@marvell.com>; Ruifeng.Wang@arm.com;
> > > nd@arm.com; Bruce Richardson <bruce.richardson@intel.com>;
> > > dev@dpdk.org
> > > Subject: Re: [EXT] Re: [PATCH 1/2] config/arm: avoid mcpu and march
> > > conflicts
> > >
> > > On Mon, Jan 22, 2024 at 12:54 PM Pavan Nikhilesh Bhagavatula
> > > <pbhagavatula@marvell.com> wrote:
> > > >
> > > > > On Sun, Jan 21, 2024 at 10:37 AM <pbhagavatula@marvell.com>
> wrote:
> > > > > >
> > > > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > >
> > > > > > The compiler options march and mtune are a subset
> > > > > > of mcpu and will lead to conflicts if improper march
> > > > > > is chosen for a given mcpu.
> > > > > > To avoid conflicts, force part number march when
> > > > > > mcpu is available and is supported by the compiler.
> > > > > >
> > > > > > Example:
> > > > > > march = armv9-a
> > > > > > mcpu = neoverse-n2
> > > > > >
> > > > > > mcpu supported, march supported
> > > > > > machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
> > > > > >
> > > > > > mcpu supported, march not supported
> > > > > > machine_args = ['-mcpu=neoverse-n2']
> > > > > >
> > > > > > mcpu not supported, march supported
> > > > > > machine_args = ['-march=armv9-a']
> > > > > >
> > > > > > mcpu not supported, march not supported
> > > > > > machine_args = ['-march=armv8.6-a']
> > > > > >
> > > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > > ---
> > > > > > config/arm/meson.build | 109 +++++++++++++++++++++++++-----
> ----
> > > -----
> > > > > --
> > > > > > 1 file changed, 67 insertions(+), 42 deletions(-)
> > > > > >
> > > > > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > > > > index 36f21d2259..8c8cfccca0 100644
> > > > > > --- a/config/arm/meson.build
> > > > > > +++ b/config/arm/meson.build
> > > > > <snip>
> > > > > > @@ -127,21 +128,22 @@ implementer_cavium = {
> > > > > > ],
> > > > > > 'part_number_config': {
> > > > > > '0xa1': {
> > > > > > - 'compiler_options': ['-mcpu=thunderxt88'],
> > > > > > + 'mcpu': 'thunderxt88',
> > > > > > 'flags': flags_part_number_thunderx
> > > > > > },
> > > > > > '0xa2': {
> > > > > > - 'compiler_options': ['-mcpu=thunderxt81'],
> > > > > > + 'mcpu': 'thunderxt81',
> > > > > > 'flags': flags_part_number_thunderx
> > > > > > },
> > > > > > '0xa3': {
> > > > > > - 'compiler_options': ['-march=armv8-a+crc', '-
> > > mcpu=thunderxt83'],
> > > > > > + 'mcpu': 'thunderxt83',
> > > > > > + 'compiler_options': ['-march=armv8-a+crc'],
> > > > >
> > > > > Let's unify this with the rest and specify 'march': 'armv8-a+crc'
> > > > > instead of having it under compiler_options.
> > > >
> > > > Ack.
> > > >
> > > > >
> > > > > > 'flags': flags_part_number_thunderx
> > > > > > },
> > > > > > '0xaf': {
> > > > > > 'march': 'armv8.1-a',
> > > > > > 'march_features': ['crc', 'crypto'],
> > > > > > - 'compiler_options': ['-mcpu=thunderx2t99'],
> > > > > > + 'mcpu': 'thunderx2t99',
> > > > > > 'flags': [
> > > > > > ['RTE_MACHINE', '"thunderx2"'],
> > > > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > > > @@ -153,7 +155,7 @@ implementer_cavium = {
> > > > > > '0xb2': {
> > > > > > 'march': 'armv8.2-a',
> > > > > > 'march_features': ['crc', 'crypto', 'lse'],
> > > > > > - 'compiler_options': ['-mcpu=octeontx2'],
> > > > > > + 'mcpu': 'octeontx2',
> > > > > > 'flags': [
> > > > > > ['RTE_MACHINE', '"cn9k"'],
> > > > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > > > @@ -176,7 +178,7 @@ implementer_ampere = {
> > > > > > '0x0': {
> > > > > > 'march': 'armv8-a',
> > > > > > 'march_features': ['crc', 'crypto'],
> > > > > > - 'compiler_options': ['-mtune=emag'],
> > > > > > + 'mcpu': 'emag',
> > > > >
> > > > > We're changing mtune to mcpu, is this equivalent?
> > > > >
> > > >
> > > > Both march and mtune are a subset of mcpu.
> > > >
> > >
> > > Sure, but we replaced '-mtune=emag' with '-mcpu=emag'. Are these two
> > > builds going to be different or the same?
> > >
> >
> > Yeah, I believe both are same.
> >
>
> Ok, the change is fine then.
>
> > > > > > 'flags': [
> > > > > > ['RTE_MACHINE', '"eMAG"'],
> > > > > > ['RTE_MAX_LCORE', 32],
> > > > > > @@ -186,7 +188,7 @@ implementer_ampere = {
> > > > > > '0xac3': {
> > > > > > 'march': 'armv8.6-a',
> > > > > > 'march_features': ['crc', 'crypto'],
> > > > > > - 'compiler_options': ['-mcpu=ampere1'],
> > > > > > + 'mcpu': 'ampere1',
> > > > > > 'flags': [
> > > > > > ['RTE_MACHINE', '"AmpereOne"'],
> > > > > > ['RTE_MAX_LCORE', 320],
> > > > > > @@ -206,7 +208,7 @@ implementer_hisilicon = {
> > > > > > '0xd01': {
> > > > > > 'march': 'armv8.2-a',
> > > > > > 'march_features': ['crypto'],
> > > > > > - 'compiler_options': ['-mtune=tsv110'],
> > > > > > + 'mcpu': 'tsv110',
> > > > > > 'flags': [
> > > > > > ['RTE_MACHINE', '"Kunpeng 920"'],
> > > > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > > > @@ -695,11 +697,23 @@ if update_flags
> > > > > >
> > > > > > machine_args = [] # Clear previous machine args
> > > > > >
> > > > > > + candidate_mcpu = ''
> > > > > > + support_mcpu = false
> > > > > > + if part_number_config.has_key('mcpu')
> > > > > > + mcpu = part_number_config['mcpu']
> > > > > > + if (cc.has_argument('-mcpu=' + mcpu))
> > > > > > + candidate_mcpu = mcpu
> > > > > > + support_mcpu = true
> > > > > > + endif
> > > > > > + endif
> > > > > > +
> > > > > > # probe supported archs and their features
> > > > > > candidate_march = ''
> > > > > > if part_number_config.has_key('march')
> > > > > > - if part_number_config.get('force_march', false)
> > > > > > - candidate_march = part_number_config['march']
> > > > > > + if part_number_config.get('force_march', false) or
> support_mcpu
> > > > >
> > > > > Instead of using the extra "support_mcpu" variable, we could do the
> > > > > same check as with candidate march (if candidate_mcpu != '', which we
> > > > > actually do below in the last lines of the patch).
> > > > >
> > > >
> > > > Ack.
> > > >
> > > > > If I understand the logic correctly, we don't want to do the march
> > > > > fallback if mcpu is specified - either the march works with the given
> > > > > mcpu or we do without it (because we don't actually need it with
> > > > > mcpu). Is that correct?
> > > > >
> > > >
> > > > Yes, but still exact march defined in part_number_config should be
> present
> > > for setting extra_march_features.
> > > > specially for expressing crypto support.
> > > >
> > >
> > > Ok, thanks.
> > >
> > > > > > + if cc.has_argument('-march=' + part_number_config['march'])
> > > > >
> > > > > Now that we've added mcpu into the mix, is this still the right
> > > > > condition? Can the below happen?
> > > > >
> > > > > This check finds that machine_args = ['-march=armv9-a'] is supported.
> > > > >
> > > > > But taken together with mcpu (machine_args = ['-mcpu=neoverse-n2',
> > > > > '-march=armv9-a']), it is not supported? In this case we'll end up
> > > > > with invalid configuration.
> > > >
> > > > This is the only correct option and evolves into -march=armv9-
> > > a+sve2+crypto for cn10k
> > > > whereas other neoverse-n2 might only have -march=armv9-a+sve2.
> > > >
> > >
> > > Maybe I should rephrase my question a bit:
> > >
> > > The correct options are ['-mcpu=neoverse-n2', '-march=armv9-a']. Is it
> > > possible that the compiler will say:
> > >
> > > ['-mcpu=neoverse-n2', '-march=armv9-a'] is supported
> > > ['-mcpu=neoverse-n2'] is supported
> > > ['-march=armv9-a'] is not supported
> > >
> >
> > Yes, this is possible.
> >
>
> Ok, do we want to make sure it's possible to configure the options in
> this way? But maybe that's out of scope of this patch.
>
This patch already handles those conditions, this is the case 2 mentioned in the example.
march = armv9-a
mcpu = neoverse-n2
[1] mcpu supported, march supported
machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
[2] mcpu supported, march not supported
machine_args = ['-mcpu=neoverse-n2']
[3] mcpu not supported, march supported
machine_args = ['-march=armv9-a']
[4] mcpu not supported, march not supported
machine_args = ['-march=armv8.6-a']
> > > So basically the question is are we risking that the compiler will say
> > > it supports both options only when both are passed while also saying
> > > it doesn't support one or both of them when checked alone. We've seen
> > > this behavior with newer compilers in aarch32 builds
> > > (-march=armv8-a+simd -mfpu=auto are supported when both are passed,
> > > but -march=armv8-a is not supported alone), so I wanted to be sure.
> > >
> > > > Example:
> > > >
> > > > Good:
> > > > #aarch64-linux-gnu-gcc -march=armv9-a+sve2+crypto -mcpu=neoverse-
> n2
> > > shrn.c
> > > > #aarch64-linux-gnu-gcc -march=armv9-a+sve2 -mcpu=neoverse-n2
> shrn.c
> > > > #aarch64-linux-gnu-gcc -march=armv9-a -mcpu=neoverse-n2 shrn.c
> > > >
> > > > Bad:
> > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8-a shrn.c
> > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8-
> a'
> > > switch
> > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.1-a shrn.c
> > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> march=armv8.1-a'
> > > switch
> > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.2-a shrn.c
> > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> march=armv8.2-a'
> > > switch
> > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.3-a shrn.c
> > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> march=armv8.3-a'
> > > switch
> > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.4-a shrn.c
> > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> march=armv8.4-a'
> > > switch
> > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.5-a shrn.c
> > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> march=armv8.5-a'
> > > switch
> > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c
> > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> march=armv8.6-a'
> > > switch
> > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.7-a shrn.c
> > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> march=armv8.7-a'
> > > switch
> > > >
> > > > >
> > > > > > + candidate_march = part_number_config['march']
> > > > > > + endif
> > > > > > else
> > > > > > supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a',
> > > 'armv8.3-
> > > > > a',
> > > > > > 'armv8.2-a', 'armv8.1-a', 'armv8-a']
On Tue, Jan 30, 2024 at 5:16 PM Pavan Nikhilesh Bhagavatula
<pbhagavatula@marvell.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Sent: Monday, January 29, 2024 2:15 PM
> > To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > Cc: Jerin Jacob <jerinj@marvell.com>; Ruifeng.Wang@arm.com;
> > nd@arm.com; Bruce Richardson <bruce.richardson@intel.com>;
> > dev@dpdk.org
> > Subject: Re: [EXT] Re: [PATCH 1/2] config/arm: avoid mcpu and march
> > conflicts
> >
> > On Wed, Jan 24, 2024 at 4:22 PM Pavan Nikhilesh Bhagavatula
> > <pbhagavatula@marvell.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > Sent: Monday, January 22, 2024 9:57 PM
> > > > To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > > > Cc: Jerin Jacob <jerinj@marvell.com>; Ruifeng.Wang@arm.com;
> > > > nd@arm.com; Bruce Richardson <bruce.richardson@intel.com>;
> > > > dev@dpdk.org
> > > > Subject: Re: [EXT] Re: [PATCH 1/2] config/arm: avoid mcpu and march
> > > > conflicts
> > > >
> > > > On Mon, Jan 22, 2024 at 12:54 PM Pavan Nikhilesh Bhagavatula
> > > > <pbhagavatula@marvell.com> wrote:
> > > > >
> > > > > > On Sun, Jan 21, 2024 at 10:37 AM <pbhagavatula@marvell.com>
> > wrote:
> > > > > > >
> > > > > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > > >
> > > > > > > The compiler options march and mtune are a subset
> > > > > > > of mcpu and will lead to conflicts if improper march
> > > > > > > is chosen for a given mcpu.
> > > > > > > To avoid conflicts, force part number march when
> > > > > > > mcpu is available and is supported by the compiler.
> > > > > > >
> > > > > > > Example:
> > > > > > > march = armv9-a
> > > > > > > mcpu = neoverse-n2
> > > > > > >
> > > > > > > mcpu supported, march supported
> > > > > > > machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
> > > > > > >
> > > > > > > mcpu supported, march not supported
> > > > > > > machine_args = ['-mcpu=neoverse-n2']
> > > > > > >
> > > > > > > mcpu not supported, march supported
> > > > > > > machine_args = ['-march=armv9-a']
> > > > > > >
> > > > > > > mcpu not supported, march not supported
> > > > > > > machine_args = ['-march=armv8.6-a']
> > > > > > >
> > > > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > > > ---
> > > > > > > config/arm/meson.build | 109 +++++++++++++++++++++++++-----
> > ----
> > > > -----
> > > > > > --
> > > > > > > 1 file changed, 67 insertions(+), 42 deletions(-)
> > > > > > >
> > > > > > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > > > > > index 36f21d2259..8c8cfccca0 100644
> > > > > > > --- a/config/arm/meson.build
> > > > > > > +++ b/config/arm/meson.build
> > > > > > <snip>
> > > > > > > @@ -127,21 +128,22 @@ implementer_cavium = {
> > > > > > > ],
> > > > > > > 'part_number_config': {
> > > > > > > '0xa1': {
> > > > > > > - 'compiler_options': ['-mcpu=thunderxt88'],
> > > > > > > + 'mcpu': 'thunderxt88',
> > > > > > > 'flags': flags_part_number_thunderx
> > > > > > > },
> > > > > > > '0xa2': {
> > > > > > > - 'compiler_options': ['-mcpu=thunderxt81'],
> > > > > > > + 'mcpu': 'thunderxt81',
> > > > > > > 'flags': flags_part_number_thunderx
> > > > > > > },
> > > > > > > '0xa3': {
> > > > > > > - 'compiler_options': ['-march=armv8-a+crc', '-
> > > > mcpu=thunderxt83'],
> > > > > > > + 'mcpu': 'thunderxt83',
> > > > > > > + 'compiler_options': ['-march=armv8-a+crc'],
> > > > > >
> > > > > > Let's unify this with the rest and specify 'march': 'armv8-a+crc'
> > > > > > instead of having it under compiler_options.
> > > > >
> > > > > Ack.
> > > > >
> > > > > >
> > > > > > > 'flags': flags_part_number_thunderx
> > > > > > > },
> > > > > > > '0xaf': {
> > > > > > > 'march': 'armv8.1-a',
> > > > > > > 'march_features': ['crc', 'crypto'],
> > > > > > > - 'compiler_options': ['-mcpu=thunderx2t99'],
> > > > > > > + 'mcpu': 'thunderx2t99',
> > > > > > > 'flags': [
> > > > > > > ['RTE_MACHINE', '"thunderx2"'],
> > > > > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > > > > @@ -153,7 +155,7 @@ implementer_cavium = {
> > > > > > > '0xb2': {
> > > > > > > 'march': 'armv8.2-a',
> > > > > > > 'march_features': ['crc', 'crypto', 'lse'],
> > > > > > > - 'compiler_options': ['-mcpu=octeontx2'],
> > > > > > > + 'mcpu': 'octeontx2',
> > > > > > > 'flags': [
> > > > > > > ['RTE_MACHINE', '"cn9k"'],
> > > > > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > > > > @@ -176,7 +178,7 @@ implementer_ampere = {
> > > > > > > '0x0': {
> > > > > > > 'march': 'armv8-a',
> > > > > > > 'march_features': ['crc', 'crypto'],
> > > > > > > - 'compiler_options': ['-mtune=emag'],
> > > > > > > + 'mcpu': 'emag',
> > > > > >
> > > > > > We're changing mtune to mcpu, is this equivalent?
> > > > > >
> > > > >
> > > > > Both march and mtune are a subset of mcpu.
> > > > >
> > > >
> > > > Sure, but we replaced '-mtune=emag' with '-mcpu=emag'. Are these two
> > > > builds going to be different or the same?
> > > >
> > >
> > > Yeah, I believe both are same.
> > >
> >
> > Ok, the change is fine then.
> >
> > > > > > > 'flags': [
> > > > > > > ['RTE_MACHINE', '"eMAG"'],
> > > > > > > ['RTE_MAX_LCORE', 32],
> > > > > > > @@ -186,7 +188,7 @@ implementer_ampere = {
> > > > > > > '0xac3': {
> > > > > > > 'march': 'armv8.6-a',
> > > > > > > 'march_features': ['crc', 'crypto'],
> > > > > > > - 'compiler_options': ['-mcpu=ampere1'],
> > > > > > > + 'mcpu': 'ampere1',
> > > > > > > 'flags': [
> > > > > > > ['RTE_MACHINE', '"AmpereOne"'],
> > > > > > > ['RTE_MAX_LCORE', 320],
> > > > > > > @@ -206,7 +208,7 @@ implementer_hisilicon = {
> > > > > > > '0xd01': {
> > > > > > > 'march': 'armv8.2-a',
> > > > > > > 'march_features': ['crypto'],
> > > > > > > - 'compiler_options': ['-mtune=tsv110'],
> > > > > > > + 'mcpu': 'tsv110',
> > > > > > > 'flags': [
> > > > > > > ['RTE_MACHINE', '"Kunpeng 920"'],
> > > > > > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > > > > > @@ -695,11 +697,23 @@ if update_flags
> > > > > > >
> > > > > > > machine_args = [] # Clear previous machine args
> > > > > > >
> > > > > > > + candidate_mcpu = ''
> > > > > > > + support_mcpu = false
> > > > > > > + if part_number_config.has_key('mcpu')
> > > > > > > + mcpu = part_number_config['mcpu']
> > > > > > > + if (cc.has_argument('-mcpu=' + mcpu))
> > > > > > > + candidate_mcpu = mcpu
> > > > > > > + support_mcpu = true
> > > > > > > + endif
> > > > > > > + endif
> > > > > > > +
> > > > > > > # probe supported archs and their features
> > > > > > > candidate_march = ''
> > > > > > > if part_number_config.has_key('march')
> > > > > > > - if part_number_config.get('force_march', false)
> > > > > > > - candidate_march = part_number_config['march']
> > > > > > > + if part_number_config.get('force_march', false) or
> > support_mcpu
> > > > > >
> > > > > > Instead of using the extra "support_mcpu" variable, we could do the
> > > > > > same check as with candidate march (if candidate_mcpu != '', which we
> > > > > > actually do below in the last lines of the patch).
> > > > > >
> > > > >
> > > > > Ack.
> > > > >
> > > > > > If I understand the logic correctly, we don't want to do the march
> > > > > > fallback if mcpu is specified - either the march works with the given
> > > > > > mcpu or we do without it (because we don't actually need it with
> > > > > > mcpu). Is that correct?
> > > > > >
> > > > >
> > > > > Yes, but still exact march defined in part_number_config should be
> > present
> > > > for setting extra_march_features.
> > > > > specially for expressing crypto support.
> > > > >
> > > >
> > > > Ok, thanks.
> > > >
> > > > > > > + if cc.has_argument('-march=' + part_number_config['march'])
> > > > > >
> > > > > > Now that we've added mcpu into the mix, is this still the right
> > > > > > condition? Can the below happen?
> > > > > >
> > > > > > This check finds that machine_args = ['-march=armv9-a'] is supported.
> > > > > >
> > > > > > But taken together with mcpu (machine_args = ['-mcpu=neoverse-n2',
> > > > > > '-march=armv9-a']), it is not supported? In this case we'll end up
> > > > > > with invalid configuration.
> > > > >
> > > > > This is the only correct option and evolves into -march=armv9-
> > > > a+sve2+crypto for cn10k
> > > > > whereas other neoverse-n2 might only have -march=armv9-a+sve2.
> > > > >
> > > >
> > > > Maybe I should rephrase my question a bit:
> > > >
> > > > The correct options are ['-mcpu=neoverse-n2', '-march=armv9-a']. Is it
> > > > possible that the compiler will say:
> > > >
> > > > ['-mcpu=neoverse-n2', '-march=armv9-a'] is supported
> > > > ['-mcpu=neoverse-n2'] is supported
> > > > ['-march=armv9-a'] is not supported
> > > >
> > >
> > > Yes, this is possible.
> > >
> >
> > Ok, do we want to make sure it's possible to configure the options in
> > this way? But maybe that's out of scope of this patch.
> >
>
> This patch already handles those conditions, this is the case 2 mentioned in the example.
>
> march = armv9-a
> mcpu = neoverse-n2
>
> [1] mcpu supported, march supported
> machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
>
> [2] mcpu supported, march not supported
> machine_args = ['-mcpu=neoverse-n2']
>
> [3] mcpu not supported, march supported
> machine_args = ['-march=armv9-a']
>
> [4] mcpu not supported, march not supported
> machine_args = ['-march=armv8.6-a']
>
Ok, that sounds great, thanks.
> > > > So basically the question is are we risking that the compiler will say
> > > > it supports both options only when both are passed while also saying
> > > > it doesn't support one or both of them when checked alone. We've seen
> > > > this behavior with newer compilers in aarch32 builds
> > > > (-march=armv8-a+simd -mfpu=auto are supported when both are passed,
> > > > but -march=armv8-a is not supported alone), so I wanted to be sure.
> > > >
> > > > > Example:
> > > > >
> > > > > Good:
> > > > > #aarch64-linux-gnu-gcc -march=armv9-a+sve2+crypto -mcpu=neoverse-
> > n2
> > > > shrn.c
> > > > > #aarch64-linux-gnu-gcc -march=armv9-a+sve2 -mcpu=neoverse-n2
> > shrn.c
> > > > > #aarch64-linux-gnu-gcc -march=armv9-a -mcpu=neoverse-n2 shrn.c
> > > > >
> > > > > Bad:
> > > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8-a shrn.c
> > > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8-
> > a'
> > > > switch
> > > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.1-a shrn.c
> > > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> > march=armv8.1-a'
> > > > switch
> > > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.2-a shrn.c
> > > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> > march=armv8.2-a'
> > > > switch
> > > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.3-a shrn.c
> > > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> > march=armv8.3-a'
> > > > switch
> > > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.4-a shrn.c
> > > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> > march=armv8.4-a'
> > > > switch
> > > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.5-a shrn.c
> > > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> > march=armv8.5-a'
> > > > switch
> > > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c
> > > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> > march=armv8.6-a'
> > > > switch
> > > > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.7-a shrn.c
> > > > > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-
> > march=armv8.7-a'
> > > > switch
> > > > >
> > > > > >
> > > > > > > + candidate_march = part_number_config['march']
> > > > > > > + endif
> > > > > > > else
> > > > > > > supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a',
> > > > 'armv8.3-
> > > > > > a',
> > > > > > > 'armv8.2-a', 'armv8.1-a', 'armv8-a']
@@ -58,18 +58,18 @@ implementer_generic = {
}
part_number_config_arm = {
- '0xd03': {'compiler_options': ['-mcpu=cortex-a53']},
- '0xd04': {'compiler_options': ['-mcpu=cortex-a35']},
- '0xd05': {'compiler_options': ['-mcpu=cortex-a55']},
- '0xd07': {'compiler_options': ['-mcpu=cortex-a57']},
- '0xd08': {'compiler_options': ['-mcpu=cortex-a72']},
- '0xd09': {'compiler_options': ['-mcpu=cortex-a73']},
- '0xd0a': {'compiler_options': ['-mcpu=cortex-a75']},
- '0xd0b': {'compiler_options': ['-mcpu=cortex-a76']},
+ '0xd03': {'mcpu': 'cortex-a53'},
+ '0xd04': {'mcpu': 'cortex-a35'},
+ '0xd05': {'mcpu': 'cortex-a55'},
+ '0xd07': {'mcpu': 'cortex-a57'},
+ '0xd08': {'mcpu': 'cortex-a72'},
+ '0xd09': {'mcpu': 'cortex-a73'},
+ '0xd0a': {'mcpu': 'cortex-a75'},
+ '0xd0b': {'mcpu': 'cortex-a76'},
'0xd0c': {
'march': 'armv8.2-a',
'march_features': ['crypto', 'rcpc'],
- 'compiler_options': ['-mcpu=neoverse-n1'],
+ 'mcpu': 'neoverse-n1',
'flags': [
['RTE_MACHINE', '"neoverse-n1"'],
['RTE_ARM_FEATURE_ATOMICS', true],
@@ -81,7 +81,7 @@ part_number_config_arm = {
'0xd40': {
'march': 'armv8.4-a',
'march_features': ['sve'],
- 'compiler_options': ['-mcpu=neoverse-v1'],
+ 'mcpu': 'neoverse-v1',
'flags': [
['RTE_MACHINE', '"neoverse-v1"'],
['RTE_ARM_FEATURE_ATOMICS', true],
@@ -92,8 +92,9 @@ part_number_config_arm = {
'march': 'armv8.4-a',
},
'0xd49': {
+ 'march': 'armv9-a',
'march_features': ['sve2'],
- 'compiler_options': ['-mcpu=neoverse-n2'],
+ 'mcpu': 'neoverse-n2',
'flags': [
['RTE_MACHINE', '"neoverse-n2"'],
['RTE_ARM_FEATURE_ATOMICS', true],
@@ -127,21 +128,22 @@ implementer_cavium = {
],
'part_number_config': {
'0xa1': {
- 'compiler_options': ['-mcpu=thunderxt88'],
+ 'mcpu': 'thunderxt88',
'flags': flags_part_number_thunderx
},
'0xa2': {
- 'compiler_options': ['-mcpu=thunderxt81'],
+ 'mcpu': 'thunderxt81',
'flags': flags_part_number_thunderx
},
'0xa3': {
- 'compiler_options': ['-march=armv8-a+crc', '-mcpu=thunderxt83'],
+ 'mcpu': 'thunderxt83',
+ 'compiler_options': ['-march=armv8-a+crc'],
'flags': flags_part_number_thunderx
},
'0xaf': {
'march': 'armv8.1-a',
'march_features': ['crc', 'crypto'],
- 'compiler_options': ['-mcpu=thunderx2t99'],
+ 'mcpu': 'thunderx2t99',
'flags': [
['RTE_MACHINE', '"thunderx2"'],
['RTE_ARM_FEATURE_ATOMICS', true],
@@ -153,7 +155,7 @@ implementer_cavium = {
'0xb2': {
'march': 'armv8.2-a',
'march_features': ['crc', 'crypto', 'lse'],
- 'compiler_options': ['-mcpu=octeontx2'],
+ 'mcpu': 'octeontx2',
'flags': [
['RTE_MACHINE', '"cn9k"'],
['RTE_ARM_FEATURE_ATOMICS', true],
@@ -176,7 +178,7 @@ implementer_ampere = {
'0x0': {
'march': 'armv8-a',
'march_features': ['crc', 'crypto'],
- 'compiler_options': ['-mtune=emag'],
+ 'mcpu': 'emag',
'flags': [
['RTE_MACHINE', '"eMAG"'],
['RTE_MAX_LCORE', 32],
@@ -186,7 +188,7 @@ implementer_ampere = {
'0xac3': {
'march': 'armv8.6-a',
'march_features': ['crc', 'crypto'],
- 'compiler_options': ['-mcpu=ampere1'],
+ 'mcpu': 'ampere1',
'flags': [
['RTE_MACHINE', '"AmpereOne"'],
['RTE_MAX_LCORE', 320],
@@ -206,7 +208,7 @@ implementer_hisilicon = {
'0xd01': {
'march': 'armv8.2-a',
'march_features': ['crypto'],
- 'compiler_options': ['-mtune=tsv110'],
+ 'mcpu': 'tsv110',
'flags': [
['RTE_MACHINE', '"Kunpeng 920"'],
['RTE_ARM_FEATURE_ATOMICS', true],
@@ -695,11 +697,23 @@ if update_flags
machine_args = [] # Clear previous machine args
+ candidate_mcpu = ''
+ support_mcpu = false
+ if part_number_config.has_key('mcpu')
+ mcpu = part_number_config['mcpu']
+ if (cc.has_argument('-mcpu=' + mcpu))
+ candidate_mcpu = mcpu
+ support_mcpu = true
+ endif
+ endif
+
# probe supported archs and their features
candidate_march = ''
if part_number_config.has_key('march')
- if part_number_config.get('force_march', false)
- candidate_march = part_number_config['march']
+ if part_number_config.get('force_march', false) or support_mcpu
+ if cc.has_argument('-march=' + part_number_config['march'])
+ candidate_march = part_number_config['march']
+ endif
else
supported_marchs = ['armv8.6-a', 'armv8.5-a', 'armv8.4-a', 'armv8.3-a',
'armv8.2-a', 'armv8.1-a', 'armv8-a']
@@ -717,32 +731,43 @@ if update_flags
endif
endforeach
endif
- if candidate_march == ''
- error('No suitable armv8 march version found.')
- endif
+
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))
+ warning('Configuration march version is @0@, not supported.'
+ .format(part_number_config['march']))
+ if candidate_march != ''
+ warning('Using march version @0@.'.format(candidate_march))
+ endif
endif
- candidate_march = '-march=' + candidate_march
- march_features = []
- if part_number_config.has_key('march_features')
- march_features += part_number_config['march_features']
+ if candidate_march == '' and candidate_mcpu == ''
+ error('No suitable ARM march/mcpu version found.')
endif
- if soc_config.has_key('extra_march_features')
- march_features += soc_config['extra_march_features']
- endif
- foreach feature: march_features
- if cc.has_argument('+'.join([candidate_march, feature]))
- candidate_march = '+'.join([candidate_march, feature])
- else
- warning('The compiler does not support feature @0@'
- .format(feature))
+
+ if candidate_march != ''
+ candidate_march = '-march=' + candidate_march
+ march_features = []
+ if part_number_config.has_key('march_features')
+ march_features += part_number_config['march_features']
endif
- endforeach
- machine_args += candidate_march
+ if soc_config.has_key('extra_march_features')
+ march_features += soc_config['extra_march_features']
+ endif
+ foreach feature: march_features
+ if cc.has_argument('+'.join([candidate_march, feature]))
+ candidate_march = '+'.join([candidate_march, feature])
+ else
+ warning('The compiler does not support feature @0@'
+ .format(feature))
+ endif
+ endforeach
+ machine_args += candidate_march
+ endif
+ endif
+
+ if candidate_mcpu != ''
+ candidate_mcpu = '-mcpu=' + candidate_mcpu
+ machine_args += candidate_mcpu
endif
# apply supported compiler options