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 BE249428C6; Tue, 4 Apr 2023 18:39:25 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4AF4D40EE3; Tue, 4 Apr 2023 18:39:25 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 16E4940A7E for ; Tue, 4 Apr 2023 18:39:24 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id E5170210DDA5; Tue, 4 Apr 2023 09:39:22 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com E5170210DDA5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1680626362; bh=5+RJy+KJoXUXTGQoLLRLU3Kjfq3GcEllHUm6kA1STHw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PlWO3VrQZQGBze4G54yANM5ArwKf+rNh/VNedvIy93LE8WdBAzJlrqrWy5/AivvFk 0UFXGWTUeEltItPfKYSVEx3vuwI2jtd5PqGEwOIiFHt1Tj0Dg8Uk37Qu/duRhWNzE9 wNVQPNm3zjNtb+/PHsTbqHgd3TaUik+dzDU3jct4= Date: Tue, 4 Apr 2023 09:39:22 -0700 From: Tyler Retzlaff To: Bruce Richardson Cc: dev@dpdk.org, david.marchand@redhat.com, thomas@monjalon.net, mb@smartsharesystems.com Subject: Re: [PATCH 3/9] eal: use barrier intrinsics when compiling with msvc Message-ID: <20230404163922.GD18560@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1680558751-17931-1-git-send-email-roretzla@linux.microsoft.com> <1680558751-17931-4-git-send-email-roretzla@linux.microsoft.com> <20230404154301.GB23247@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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, Apr 04, 2023 at 05:23:07PM +0100, Bruce Richardson wrote: > On Tue, Apr 04, 2023 at 08:43:01AM -0700, Tyler Retzlaff wrote: > > On Tue, Apr 04, 2023 at 09:53:21AM +0100, Bruce Richardson wrote: > > > On Mon, Apr 03, 2023 at 02:52:25PM -0700, Tyler Retzlaff wrote: > > > > Inline assembly is not supported for msvc x64 instead use > > > > _{Read,Write,ReadWrite}Barrier() intrinsics. > > > > > > > > Signed-off-by: Tyler Retzlaff > > > > --- > > > > lib/eal/include/generic/rte_atomic.h | 4 ++++ > > > > lib/eal/x86/include/rte_atomic.h | 10 +++++++++- > > > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/lib/eal/include/generic/rte_atomic.h b/lib/eal/include/generic/rte_atomic.h > > > > index 234b268..e973184 100644 > > > > --- a/lib/eal/include/generic/rte_atomic.h > > > > +++ b/lib/eal/include/generic/rte_atomic.h > > > > @@ -116,9 +116,13 @@ > > > > * Guarantees that operation reordering does not occur at compile time > > > > * for operations directly before and after the barrier. > > > > */ > > > > +#ifndef RTE_TOOLCHAIN_MSVC > > > > #define rte_compiler_barrier() do { \ > > > > asm volatile ("" : : : "memory"); \ > > > > } while(0) > > > > +#else > > > > +#define rte_compiler_barrier() _ReadWriteBarrier() > > > > > > Does this actually add a full memory barrier? If so, that's really not what we > > > want, and will slow things down. > > > > for background MSVC when targeting amd64/arm64 do not permit inline > > assmebly. The main reason is inline assembly can't be optimized. > > instead it provides intrinsics (that are known) that can participate in > > optimization. > > > > specific answer to your question. yes, it implements only a "compiler > > barrier" not a full memory barrier preventing processor reordering. > > > > https://learn.microsoft.com/en-us/cpp/intrinsics/readwritebarrier?view=msvc-170 > > "Limits the compiler optimizations that can reorder memory accesses > > across the point of the call." > > > > note: ignore the caution on the documentation it only applies to C++ > > Thanks for clarifying. In that case, I think we need a different > macro/barrier for the rte_smp_mp() case. When mixing reads and writes on > x86, there are cases where we actually do need a full memory > barrier/mfence, rather than just a compiler barrier. yes, unfortunately i got distracted by expansion of rte_smp_{w,r}mp() macros in lib/eal/x86/include/rte_atomic.h and assumed they were compiler barriers. i can see now i should have been looking at the documentation comments of the inline function prototypes in lib/eal/include/generic/rte_atomic.h > > /Bruce