[PATCH 3/9] eal: use barrier intrinsics when compiling with msvc

[Tyler Retzlaff]

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 <roretzla at linux.microsoft.com>
> > > > ---
> > > >  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