From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7946E439FA; Mon, 29 Jan 2024 09:44:56 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F344240A8A; Mon, 29 Jan 2024 09:44:55 +0100 (CET) Received: from mail-ej1-f42.google.com (mail-ej1-f42.google.com [209.85.218.42]) by mails.dpdk.org (Postfix) with ESMTP id 1D50B40A6E for ; Mon, 29 Jan 2024 09:44:54 +0100 (CET) Received: by mail-ej1-f42.google.com with SMTP id a640c23a62f3a-a271a28aeb4so268161066b.2 for ; Mon, 29 Jan 2024 00:44:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1706517894; x=1707122694; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=AWjlE8dVPS16H9U/hCdZPwEo8dvtwMauXNrxDgzcO4Q=; b=aJGYLyzZ4kDDwqgBieln2KY3NyOiGJWC/bi0XLpuxY3zTnDmbipZZUwdaOCfH+Dlp0 cXlwAT100hwE2u+3RMlj7CgjLkZPR1KlkEcudGf5dDJJFWt24RRdVVfmgBqp5jt2jPk4 H1UaydiGpOZiXH6xSas1MSVxqUeCM1/+v2CbqwOaomNBjYD6HWRQmofGJRZVsKAWrvkd LbwmyU8+B5Nv1D0BIzVQovE/3zmx862+1+LX7xmmbpuipfOgoXZ5Lv91sGsYipJabgMo L0VREeMzEryTXoKv85qQQVtN0OJPXudZIaCPA/eXlrQNyWtFB2gbgmFtqEyPYzzh2puL FXEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706517894; x=1707122694; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=AWjlE8dVPS16H9U/hCdZPwEo8dvtwMauXNrxDgzcO4Q=; b=NMO53ni9PCb05gm0R5cNXZHPGncDs652e/0+jjISPsJsvQ50C4HZ0pJshYnndUyW0h V4zN2i/dgpbHMdAOjrU2VipUGl4uLPG6sl9roZGTrxbheGAblPKoEpEnW6lhVxx6zAvT eSy+Q1qkfOrvErLH0yX1nfwxaoXpMKcMKfR9fNnzcTthSa96vmR2k9FoSVQVX46rI2WV hfpIeXcz9mj1AinHP+pVr43a4atu2+k7CwBiYrkFMLk2QyVjky/uw+2LBQX91saJFdJu afqg10CL93lErK8tKwLNxxq91E9Bg7zG9+BkaqoKO4wX+tyA4Q4VRpS/+mLyFb1WoeCq ko4g== X-Gm-Message-State: AOJu0YzoLZLSG2GK2szO5ZW891MNGE4HxDOOBjuyjQN2AMAYdi59p27J 76R43JY//TbjfU0V/qGBYZ32UabfqFzpZBrjdeFXBrSj+RIBJJRLrTdZXSrBeFiZrHFPOwGs99q erxh0OzWnUcJ5f3GpKQRExZyyfntGjojXagCQVw== X-Google-Smtp-Source: AGHT+IE5zJgB4f50uglJZiR1cWKBnrgZAZICXhxMhdgNp6gUrdAC8Qse0IXEmimxB69iN1keuB48JvVqNenxRLV/JT4= X-Received: by 2002:a17:906:d7b1:b0:a35:c371:8b27 with SMTP id pk17-20020a170906d7b100b00a35c3718b27mr959830ejb.30.1706517893732; Mon, 29 Jan 2024 00:44:53 -0800 (PST) MIME-Version: 1.0 References: <20240121093653.2890-1-pbhagavatula@marvell.com> In-Reply-To: From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Mon, 29 Jan 2024 09:44:42 +0100 Message-ID: Subject: Re: [EXT] Re: [PATCH 1/2] config/arm: avoid mcpu and march conflicts To: Pavan Nikhilesh Bhagavatula Cc: Jerin Jacob , "Ruifeng.Wang@arm.com" , "nd@arm.com" , Bruce Richardson , "dev@dpdk.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Jan 24, 2024 at 4:22=E2=80=AFPM Pavan Nikhilesh Bhagavatula wrote: > > > > > -----Original Message----- > > From: Juraj Linke=C5=A1 > > Sent: Monday, January 22, 2024 9:57 PM > > To: Pavan Nikhilesh Bhagavatula > > Cc: Jerin Jacob ; Ruifeng.Wang@arm.com; > > nd@arm.com; Bruce Richardson ; > > 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=E2=80=AFPM Pavan Nikhilesh Bhagavatula > > wrote: > > > > > > > On Sun, Jan 21, 2024 at 10:37=E2=80=AFAM = wrote: > > > > > > > > > > From: Pavan Nikhilesh > > > > > > > > > > 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 =3D armv9-a > > > > > mcpu =3D neoverse-n2 > > > > > > > > > > mcpu supported, march supported > > > > > machine_args =3D ['-mcpu=3Dneoverse-n2', '-march=3Darmv9-= a'] > > > > > > > > > > mcpu supported, march not supported > > > > > machine_args =3D ['-mcpu=3Dneoverse-n2'] > > > > > > > > > > mcpu not supported, march supported > > > > > machine_args =3D ['-march=3Darmv9-a'] > > > > > > > > > > mcpu not supported, march not supported > > > > > machine_args =3D ['-march=3Darmv8.6-a'] > > > > > > > > > > Signed-off-by: Pavan Nikhilesh > > > > > --- > > > > > 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 > > > > > > > > > @@ -127,21 +128,22 @@ implementer_cavium =3D { > > > > > ], > > > > > 'part_number_config': { > > > > > '0xa1': { > > > > > - 'compiler_options': ['-mcpu=3Dthunderxt88'], > > > > > + 'mcpu': 'thunderxt88', > > > > > 'flags': flags_part_number_thunderx > > > > > }, > > > > > '0xa2': { > > > > > - 'compiler_options': ['-mcpu=3Dthunderxt81'], > > > > > + 'mcpu': 'thunderxt81', > > > > > 'flags': flags_part_number_thunderx > > > > > }, > > > > > '0xa3': { > > > > > - 'compiler_options': ['-march=3Darmv8-a+crc', '- > > mcpu=3Dthunderxt83'], > > > > > + 'mcpu': 'thunderxt83', > > > > > + 'compiler_options': ['-march=3Darmv8-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=3Dthunderx2t99'], > > > > > + 'mcpu': 'thunderx2t99', > > > > > 'flags': [ > > > > > ['RTE_MACHINE', '"thunderx2"'], > > > > > ['RTE_ARM_FEATURE_ATOMICS', true], > > > > > @@ -153,7 +155,7 @@ implementer_cavium =3D { > > > > > '0xb2': { > > > > > 'march': 'armv8.2-a', > > > > > 'march_features': ['crc', 'crypto', 'lse'], > > > > > - 'compiler_options': ['-mcpu=3Docteontx2'], > > > > > + 'mcpu': 'octeontx2', > > > > > 'flags': [ > > > > > ['RTE_MACHINE', '"cn9k"'], > > > > > ['RTE_ARM_FEATURE_ATOMICS', true], > > > > > @@ -176,7 +178,7 @@ implementer_ampere =3D { > > > > > '0x0': { > > > > > 'march': 'armv8-a', > > > > > 'march_features': ['crc', 'crypto'], > > > > > - 'compiler_options': ['-mtune=3Demag'], > > > > > + '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=3Demag' with '-mcpu=3Demag'. Are these tw= o > > 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 =3D { > > > > > '0xac3': { > > > > > 'march': 'armv8.6-a', > > > > > 'march_features': ['crc', 'crypto'], > > > > > - 'compiler_options': ['-mcpu=3Dampere1'], > > > > > + 'mcpu': 'ampere1', > > > > > 'flags': [ > > > > > ['RTE_MACHINE', '"AmpereOne"'], > > > > > ['RTE_MAX_LCORE', 320], > > > > > @@ -206,7 +208,7 @@ implementer_hisilicon =3D { > > > > > '0xd01': { > > > > > 'march': 'armv8.2-a', > > > > > 'march_features': ['crypto'], > > > > > - 'compiler_options': ['-mtune=3Dtsv110'], > > > > > + 'mcpu': 'tsv110', > > > > > 'flags': [ > > > > > ['RTE_MACHINE', '"Kunpeng 920"'], > > > > > ['RTE_ARM_FEATURE_ATOMICS', true], > > > > > @@ -695,11 +697,23 @@ if update_flags > > > > > > > > > > machine_args =3D [] # Clear previous machine args > > > > > > > > > > + candidate_mcpu =3D '' > > > > > + support_mcpu =3D false > > > > > + if part_number_config.has_key('mcpu') > > > > > + mcpu =3D part_number_config['mcpu'] > > > > > + if (cc.has_argument('-mcpu=3D' + mcpu)) > > > > > + candidate_mcpu =3D mcpu > > > > > + support_mcpu =3D true > > > > > + endif > > > > > + endif > > > > > + > > > > > # probe supported archs and their features > > > > > candidate_march =3D '' > > > > > if part_number_config.has_key('march') > > > > > - if part_number_config.get('force_march', false) > > > > > - candidate_march =3D part_number_config['march'] > > > > > + if part_number_config.get('force_march', false) or suppo= rt_mcpu > > > > > > > > Instead of using the extra "support_mcpu" variable, we could do the > > > > same check as with candidate march (if candidate_mcpu !=3D '', whic= h 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 giv= en > > > > 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 pr= esent > > for setting extra_march_features. > > > specially for expressing crypto support. > > > > > > > Ok, thanks. > > > > > > > + if cc.has_argument('-march=3D' + 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 =3D ['-march=3Darmv9-a'] is supp= orted. > > > > > > > > But taken together with mcpu (machine_args =3D ['-mcpu=3Dneoverse-n= 2', > > > > '-march=3Darmv9-a']), it is not supported? In this case we'll end u= p > > > > with invalid configuration. > > > > > > This is the only correct option and evolves into -march=3Darmv9- > > a+sve2+crypto for cn10k > > > whereas other neoverse-n2 might only have -march=3Darmv9-a+sve2. > > > > > > > Maybe I should rephrase my question a bit: > > > > The correct options are ['-mcpu=3Dneoverse-n2', '-march=3Darmv9-a']. Is= it > > possible that the compiler will say: > > > > ['-mcpu=3Dneoverse-n2', '-march=3Darmv9-a'] is supported > > ['-mcpu=3Dneoverse-n2'] is supported > > ['-march=3Darmv9-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=3Darmv8-a+simd -mfpu=3Dauto are supported when both are passed, > > but -march=3Darmv8-a is not supported alone), so I wanted to be sure. > > > > > Example: > > > > > > Good: > > > #aarch64-linux-gnu-gcc -march=3Darmv9-a+sve2+crypto -mcpu=3Dneoverse= -n2 > > shrn.c > > > #aarch64-linux-gnu-gcc -march=3Darmv9-a+sve2 -mcpu=3Dneoverse-n2 shr= n.c > > > #aarch64-linux-gnu-gcc -march=3Darmv9-a -mcpu=3Dneoverse-n2 shrn.c > > > > > > Bad: > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8-a shrn.c > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '-march=3Da= rmv8-a' > > switch > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.1-a shrn.c > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '-march=3Da= rmv8.1-a' > > switch > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.2-a shrn.c > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '-march=3Da= rmv8.2-a' > > switch > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.3-a shrn.c > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '-march=3Da= rmv8.3-a' > > switch > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.4-a shrn.c > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '-march=3Da= rmv8.4-a' > > switch > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.5-a shrn.c > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '-march=3Da= rmv8.5-a' > > switch > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.6-a shrn.c > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '-march=3Da= rmv8.6-a' > > switch > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.7-a shrn.c > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '-march=3Da= rmv8.7-a' > > switch > > > > > > > > > > > > + candidate_march =3D part_number_config['march'] > > > > > + endif > > > > > else > > > > > supported_marchs =3D ['armv8.6-a', 'armv8.5-a', 'arm= v8.4-a', > > 'armv8.3- > > > > a', > > > > > 'armv8.2-a', 'armv8.1-a', 'armv8= -a']