> From: Bruce Richardson [mailto:bruce.richard...@intel.com] > Sent: Friday, 25 August 2023 16.13 > > 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 <roret...@linux.microsoft.com> > > Acked-by: Morten Brørup <m...@smartsharesystems.com> > > Acked-by: Konstantin Ananyev <konstantin.v.anan...@yandex.ru> > > --- > > 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 > > 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: > > ../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=discarded-qualifiers] > 113 | _umonitor(pmc->addr); > | ~~~^~~~~~ > > 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
