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 CDD2D43A1A; Wed, 31 Jan 2024 10:03:54 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 46D7A402C5; Wed, 31 Jan 2024 10:03:54 +0100 (CET) Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by mails.dpdk.org (Postfix) with ESMTP id C6183402A1 for ; Wed, 31 Jan 2024 10:03:51 +0100 (CET) Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-a354fc17f24so452384166b.0 for ; Wed, 31 Jan 2024 01:03:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1706691831; x=1707296631; 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=U3/qNAUaht4xklSRSKih3ENEoD/sIjk9N0bzk06GDSg=; b=qXMTPz2F9RWxIufwy9Stdya2f/wyNM0aRvksSVkKVxi/lOXLzP01T1p30XTSAJHKiq ofRvYGAesR2zEzjtKTHv45fFiAxefYGscBDVWy7d/jhGKz9z+L8raSEcW/DmrFIn56Ru 02/DlWwgPbjdACAiQDaXnsBuSmn1XrT6xYfVokXMG1ms10cBtdMKhLCrCyZRjf3+gzqv eKoMEdYYG/+IHFfbj7OGVnOEcJpvuVa/XAI2hsD78/uXOxNRH9ae3EWGCdSXiIhZZrYg a4zWiD1q0o2zmypbPwHbfnYJh2voNDJCRYLR9phn5t2pwVpeQ37PcTNQ00OjC3v6Rprr 50Yw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706691831; x=1707296631; 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=U3/qNAUaht4xklSRSKih3ENEoD/sIjk9N0bzk06GDSg=; b=R6NkGM3mFZyA/qqB0aePBD6HVvmPrGnsBbrdKTYQUcD2NmjLwKhdHmAkjp8RS57eXd k6xJs3948fEHHg7hcHbJsjlTkTkscfULOFbKjcYMUhcyjwhrwlYiDH2ClV49lxrlS75Y K0mLqVh96Z1mPahQZsEhT+5TQi3M6d85a9feWHIFVl0y7BO81JSGUpDo3JvwmI9aEvo/ 0fTBlcviBPpISKiPOr7qKVb1vkgEw/0oCkTyttfoKi824MaflLogDX4BoJHsbh1sumd0 xwPKUih9kC6dLYsnHqPj49wmMoho8EwDfkHa7Xx2fJr7/FzhjhR5MLFEv19eEjeBjMOJ yaug== X-Gm-Message-State: AOJu0YzqQpHcZkyrR04mmxLj0fJtAmVMjGx6i8Uxln07CeEERKhdzJBO B2dxrhEAJoAfEekcTbfot/SgGN8tjNhjh9scowwqnwQYjhegJV5wvutg3uB54ouB4R1Cv/FYYJM OTZzlgQjwVScnLCE67zZZKllqeeeLfI/I91RfoA== X-Google-Smtp-Source: AGHT+IHdZ+BBT9diTW4ygfR59SPrE47gn7pzbmC51L4OmSUfttqc54GOT7JwRo+WYmT+CuKK0CJ2gmmYh/zVmbkBidI= X-Received: by 2002:a17:906:d0cc:b0:a2e:94a0:93b4 with SMTP id bq12-20020a170906d0cc00b00a2e94a093b4mr611426ejb.61.1706691831417; Wed, 31 Jan 2024 01:03:51 -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: Wed, 31 Jan 2024 10:03:40 +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 Tue, Jan 30, 2024 at 5:16=E2=80=AFPM Pavan Nikhilesh Bhagavatula wrote: > > > > > -----Original Message----- > > From: Juraj Linke=C5=A1 > > Sent: Monday, January 29, 2024 2:15 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 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 Bhagavatul= a > > > > 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=3Dar= mv9-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+cr= c' > > > > > > 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 thes= e 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 =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 > > support_mcpu > > > > > > > > > > > > Instead of using the extra "support_mcpu" variable, we could do= the > > > > > > same check as with candidate march (if candidate_mcpu !=3D '', = 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 ma= rch > > > > > > 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 wit= h > > > > > > mcpu). Is that correct? > > > > > > > > > > > > > > > > Yes, but still exact march defined in part_number_config should b= e > > present > > > > for setting extra_march_features. > > > > > specially for expressing crypto support. > > > > > > > > > > > > > Ok, thanks. > > > > > > > > > > > + if cc.has_argument('-march=3D' + part_number_co= nfig['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 = supported. > > > > > > > > > > > > But taken together with mcpu (machine_args =3D ['-mcpu=3Dneover= se-n2', > > > > > > '-march=3Darmv9-a']), it is not supported? In this case we'll e= nd up > > > > > > 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. > > > > This patch already handles those conditions, this is the case 2 mentioned= in the example. > > march =3D armv9-a > mcpu =3D neoverse-n2 > > [1] mcpu supported, march supported > machine_args =3D ['-mcpu=3Dneoverse-n2', '-march=3Darmv9-a'] > > [2] mcpu supported, march not supported > machine_args =3D ['-mcpu=3Dneoverse-n2'] > > [3] mcpu not supported, march supported > machine_args =3D ['-march=3Darmv9-a'] > > [4] mcpu not supported, march not supported > machine_args =3D ['-march=3Darmv8.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 sayin= g > > > > it doesn't support one or both of them when checked alone. We've se= en > > > > this behavior with newer compilers in aarch32 builds > > > > (-march=3Darmv8-a+simd -mfpu=3Dauto are supported when both are pas= sed, > > > > but -march=3Darmv8-a is not supported alone), so I wanted to be sur= e. > > > > > > > > > Example: > > > > > > > > > > Good: > > > > > #aarch64-linux-gnu-gcc -march=3Darmv9-a+sve2+crypto -mcpu=3Dneov= erse- > > n2 > > > > shrn.c > > > > > #aarch64-linux-gnu-gcc -march=3Darmv9-a+sve2 -mcpu=3Dneoverse-n2 > > shrn.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= =3Darmv8- > > a' > > > > switch > > > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.1-a shr= n.c > > > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '- > > march=3Darmv8.1-a' > > > > switch > > > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.2-a shr= n.c > > > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '- > > march=3Darmv8.2-a' > > > > switch > > > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.3-a shr= n.c > > > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '- > > march=3Darmv8.3-a' > > > > switch > > > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.4-a shr= n.c > > > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '- > > march=3Darmv8.4-a' > > > > switch > > > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.5-a shr= n.c > > > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '- > > march=3Darmv8.5-a' > > > > switch > > > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.6-a shr= n.c > > > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '- > > march=3Darmv8.6-a' > > > > switch > > > > > #aarch64-linux-gnu-gcc -mcpu=3Dneoverse-n2 -march=3Darmv8.7-a shr= n.c > > > > > cc1: warning: switch '-mcpu=3Dneoverse-n2' conflicts with '- > > march=3Darmv8.7-a' > > > > switch > > > > > > > > > > > > > > > > > > + candidate_march =3D part_number_config['marc= h'] > > > > > > > + endif > > > > > > > else > > > > > > > supported_marchs =3D ['armv8.6-a', 'armv8.5-a', = 'armv8.4-a', > > > > 'armv8.3- > > > > > > a', > > > > > > > 'armv8.2-a', 'armv8.1-a', 'a= rmv8-a']