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 0E75843101; Fri, 25 Aug 2023 16:56:57 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D82DE41140; Fri, 25 Aug 2023 16:56:56 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id D8F9740695 for ; Fri, 25 Aug 2023 16:56:55 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id B238620433; Fri, 25 Aug 2023 16:56:55 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v11 05/16] eal: use umonitor umwait and tpause intrinsics Date: Fri, 25 Aug 2023 16:56:51 +0200 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87B41@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v11 05/16] eal: use umonitor umwait and tpause intrinsics Thread-Index: AdnXXkvK4wO99wHHSuK9B0npmRoURAABVy/w References: <1680558751-17931-1-git-send-email-roretzla@linux.microsoft.com> <1691781658-32520-1-git-send-email-roretzla@linux.microsoft.com> <1691781658-32520-6-git-send-email-roretzla@linux.microsoft.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Bruce Richardson" , "Tyler Retzlaff" Cc: , "Konstantin Ananyev" , "Ciara Power" , , , 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 > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Friday, 25 August 2023 16.13 >=20 > On Fri, Aug 11, 2023 at 12:20:47PM -0700, Tyler Retzlaff wrote: > > Inline assembly is not supported for MSVC x64 instead use _umonitor, > > _umwait and _tpause intrinsics. > > > > Signed-off-by: Tyler Retzlaff > > Acked-by: Morten Br=F8rup > > Acked-by: Konstantin Ananyev > > --- > > lib/eal/x86/rte_power_intrinsics.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/lib/eal/x86/rte_power_intrinsics.c > b/lib/eal/x86/rte_power_intrinsics.c > > index f749da9..4066d13 100644 > > --- a/lib/eal/x86/rte_power_intrinsics.c > > +++ b/lib/eal/x86/rte_power_intrinsics.c > > @@ -109,9 +109,13 @@ > > */ > > > > /* set address for UMONITOR */ > > +#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__) > > + _umonitor(pmc->addr); > > +#else >=20 > This change is unfortunately giving build errors on system with = WAITPKG, > since the intrinsics do not take volatile parameters, unlike the = inline > ASM > which works fine with both volatile and non-volatile variables. This = is > the > error I see: >=20 > ../lib/eal/x86/rte_power_intrinsics.c: In function = 'rte_power_monitor': > ../lib/eal/x86/rte_power_intrinsics.c:113:22: error: passing argument = 1 > of '_umonitor' discards 'volatile' qualifier from pointer target type = [- > Werror=3Ddiscarded-qualifiers] > 113 | _umonitor(pmc->addr); > | ~~~^~~~~~ >=20 > The easy fix for now seems to be just dropping the "|| > defined(__WAITPKG__)" part of the #ifdef, and leave the intrinsic for > MSVC > only. > Any objections? I wonder if omitting the "volatile" qualifier is correct for this = parameter. Although the authors of _umonitor() apparently think so, I = have seen built-ins with questionable qualifiers before, so I wouldn't = trust it to be correct. Replacing inline assembly with built-ins is generally preferable. So I = would prefer if you typecast it away, or temporarily disable that = warning. But I don't object to the quick fix. :-) -Morten